Skip to content

Commit

Permalink
PgQuery.parse: Support complex queries with deeply nested ASTs
Browse files Browse the repository at this point in the history
Requires at least google-protobuf v3.20.0, if using older releases this
change has no effect. To avoid complex dependency updates, we do not
require google-protobuf to be current when you use this library, so you
may want to pin it to the newer version if you depend on this behaviour.

Fixes #209
  • Loading branch information
lfittl committed Apr 13, 2022
1 parent 53e39cb commit 2ecdc24
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 16 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Expand Up @@ -9,7 +9,7 @@ GEM
specs:
ast (2.4.1)
diff-lcs (1.3)
google-protobuf (3.19.4)
google-protobuf (3.20.0)
parallel (1.20.1)
parser (2.7.2.0)
ast (~> 2.4.1)
Expand Down
8 changes: 7 additions & 1 deletion lib/pg_query/parse.rb
Expand Up @@ -3,7 +3,13 @@ def self.parse(query)
result, stderr = parse_protobuf(query)

begin
result = PgQuery::ParseResult.decode(result)
result = if PgQuery::ParseResult.method(:decode).arity == 1
PgQuery::ParseResult.decode(result)
elsif PgQuery::ParseResult.method(:decode).arity == -1
PgQuery::ParseResult.decode(result, recursion_limit: 1_000)
else
raise ArgumentError, 'Unsupported protobuf Ruby API'
end
rescue Google::Protobuf::ParseError => e
raise PgQuery::ParseError.new(format('Failed to parse tree: %s', e.message), __FILE__, __LINE__, -1)
end
Expand Down
29 changes: 15 additions & 14 deletions spec/lib/parse_spec.rb
Expand Up @@ -71,20 +71,15 @@
end)
end

it 'returns parser error due to too much nesting' do
it 'parses really deep queries' do
# Old JSON test that was kept for Protobuf version (queries like this have not been seen in the real world)
query_text = 'SELECT a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(b))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))'
expect { described_class.parse(query_text) }.to(raise_error do |error|
expect(error).to be_a(PgQuery::ParseError)
expect(error.message).to start_with 'Failed to parse tree'
end)
query = described_class.parse(query_text)
expect(query.tree).not_to be_nil
expect(query.tables).to eq []
end

it 'returns parser error due to too much nesting (2)' do
# Protobuf Ruby supports a maximum message depth of 64 by default, and the
# ability to increase this is currently not exposed in the Ruby API:
# https://github.com/protocolbuffers/protobuf/issues/1493
#
it 'parses really deep queries (2)' do
# Queries like this are uncommon, but have been seen in the real world.
query_text = 'SELECT * FROM "t0"
JOIN "t1" ON (1) JOIN "t2" ON (1) JOIN "t3" ON (1) JOIN "t4" ON (1) JOIN "t5" ON (1)
Expand All @@ -93,10 +88,16 @@
JOIN "t16" ON (1) JOIN "t17" ON (1) JOIN "t18" ON (1) JOIN "t19" ON (1) JOIN "t20" ON (1)
JOIN "t21" ON (1) JOIN "t22" ON (1) JOIN "t23" ON (1) JOIN "t24" ON (1) JOIN "t25" ON (1)
JOIN "t26" ON (1) JOIN "t27" ON (1) JOIN "t28" ON (1) JOIN "t29" ON (1)'
expect { described_class.parse(query_text) }.to(raise_error do |error|
expect(error).to be_a(PgQuery::ParseError)
expect(error.message).to start_with 'Failed to parse tree'
end)
query = described_class.parse(query_text)
expect(query.tree).not_to be_nil
expect(query.tables).to eq (2..29).to_a.map {|i| "t#{i}" }.reverse + ['t0', 't1']
end

it 'parses really deep queries (3)' do
query_text = 'SELECT * FROM foo ' + (1..100).to_a.map { |i| "JOIN foo_#{i} ON foo.id = foo_#{i}.foo_id" }.join(' ')
query = described_class.parse(query_text)
expect(query.tree).not_to be_nil
expect(query.tables).to eq (2..100).to_a.map {|i| "foo_#{i}" }.reverse + ['foo', 'foo_1']
end

it "parses real queries" do
Expand Down

0 comments on commit 2ecdc24

Please sign in to comment.