New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Failed to parse tree: Error occurred during parsing #209
Comments
@kaspernj Thanks for the report! This most likely is due to the parse tree depth exceeding 64 levels, and hitting this issue in the Protobuf library: protocolbuffers/protobuf#1493 Note the C level functions work fine (i.e. What's your use case for PgQuery, could you use the functions that don't require the parse tree to be in Ruby for now? |
I am making a very dynamic API using Ransack and CanCan. Postgres has a lot of limitations in regards to distinct, order and group which requires used columns to be selected or whatever. I am using pg_query to scan the query and fix that automatically based on the used Ransack parameters (which the user can freely set as he wishes) and CanCan (which restricts which records he can read, update, destroy or whatever). This is the gem I wrote to automatically fix the Postgres queries in ActiveRecord: |
Everything worked fine in the previous version :-) |
@kaspernj Nice - thats a great use case! And unfortunately that does mean you rely on the parse tree itself - so that'd require the patch in the protobuf gem to support customizing the nesting (the API is already there internally, it just needs to be made configurable from the Ruby side). I also have plans to work on this, but a few other things on my plate right now. Two ways forward:
Generally I would recommend to focus (1), since it gets you a fully typed tree to work with, and is faster for parsing complex queries, but the downside is that it'd require diving a bit into the protobuf library. |
@lfittl Would it be possible to support getting the result as a hash again until the issue is resolved? I would love to stay on the latest version :-) |
@lfittl Are there any updates on this? :-) |
@kaspernj No time to work on this currently, though it will bubble up in priority at some point. PRs welcome though - it should be relatively straightforward to add this to pg_query_ruby.c, e.g. named as Please also add a quick test in case you work on this :) |
For what it's worth, I think this is a blocker for me too. I'm using pg_query to infer dependencies between arbitrarily long/complex SQL statements to figure out what order to execute them in. I'll need to use the parse tree in Ruby, and I expect depth will sometimes exceed 64 levels. |
This issue is still blocking us from upgrading to version 2, which we'd really like to do (there have been so many nice improvements since then!). We've confirmed several queries we currently parse exceed the 64 depth limit. We'd really need to go back to a max depth of 1000 levels. I spent a little bit of time trying to do this:
But I've never touched C before and don't have the motivation to learn/struggle through it right now. It sounds like the right idea. Is there any way I could add to someone else's motivation to tackle this issue? I'm guessing it's a fairly simple change? |
Depends on protocolbuffers/protobuf#9218, until that is merged this can only be used with a custom built protobuf gem. Fixes #209
Depends on protocolbuffers/protobuf#9218, until that is merged this can only be used with a custom built protobuf gem. Fixes #209
@Tassosb Thanks for bumping this up - I ended up investing the time to fix the root cause here, in the Ruby protobuf implementation: protocolbuffers/protobuf#9218 With that in place, the change in pg_query is rather small: #238 To test this change, you can do the following:
(note that you also need protocolbuffers/protobuf#9219 since otherwise you can't load Could you (and @kaspernj) confirm whether this fix works for your use cases? |
Depends on protocolbuffers/protobuf#9218, until that is merged this can only be used with a custom built protobuf gem. Fixes #209
Amazing! Thank you @lfittl
After running
I'm on ruby 2.7.4 with bundler 1.17.0. This issue seems maybe similar to the one addressed in PR 9219. |
HackerOne is running into this problem as well! Any updates on this? |
@mvgijssel Unfortunately we're still waiting for protocolbuffers/protobuf#9218 If you have time available, you can help with that change (it might need a rebase due to the recent ubp changes in that codebase), or investigate the JSON output method as described above. |
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
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
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
After upgrading I am suddenly starting to get these errors? There isn't much to go from in the error message.
The query is pretty long and generated using CanCan and Ransack:
The SQL doesn't throw an error when used with
Contact.find_by_sql(query)
.The text was updated successfully, but these errors were encountered: