From 2ecdc240682830b99f9c73203d664a5b595caf04 Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Fri, 12 Nov 2021 23:28:23 -0800 Subject: [PATCH] PgQuery.parse: Support complex queries with deeply nested ASTs 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 --- Gemfile.lock | 2 +- lib/pg_query/parse.rb | 8 +++++++- spec/lib/parse_spec.rb | 29 +++++++++++++++-------------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a3af0fb9..43891b2b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) diff --git a/lib/pg_query/parse.rb b/lib/pg_query/parse.rb index 704bfebe..8be4923b 100644 --- a/lib/pg_query/parse.rb +++ b/lib/pg_query/parse.rb @@ -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 diff --git a/spec/lib/parse_spec.rb b/spec/lib/parse_spec.rb index e73a1ee4..81c499c7 100644 --- a/spec/lib/parse_spec.rb +++ b/spec/lib/parse_spec.rb @@ -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) @@ -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