-
Notifications
You must be signed in to change notification settings - Fork 601
Add an AST visitor #114
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
Add an AST visitor #114
Conversation
Pull Request Test Coverage Report for Build 387
💛 - Coveralls |
73c0513
to
c71e6e2
Compare
An AST visitor would be a nice thing to have. |
Bumping this, adding an AST visitor would make it much easier to use @benesch I see you did some work on MaterializeInc/materialize#3288 in auto-generating the visitor, is this something that could potentially be upstreamed, maybe have walkabout run on |
Maybe! The build script approach, rather than the manual regeneration
approach, has been working well for us though. We could consider open
sourcing the walkabout crate on which the visitor generation depends, but I
don’t know that anyone at Materialize has much time to work on this at the
moment.
There’s an old PR from me on this repo somewhere that has a manual
implementation of the visitor, which might be of interest.
…On Tue, Mar 30, 2021 at 1:16 PM Will Eaton ***@***.***> wrote:
Bumping this, adding an AST visitor would make it much easier to use
sqlparser-rs to extract particular metadata from queries like "what
tables are referenced in this query".
@benesch <https://github.com/benesch> I see you did some work on
MaterializeInc/materialize#3288
<MaterializeInc/materialize#3288> in
auto-generating the visitor, is this something that could potentially be
upstreamed, maybe have walkabout run on pre-commit hook to force the
vistior to be regenerated?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#114 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGXSICWCA5MZAPQH3LPGJ3TGIBPZANCNFSM4HW6JRQQ>
.
|
Hi @benesch -- sorry for the delay in review. I am going to help out now with this repo and we are working to clear the backlog. Is this PR still something you would like to work on to help contribute? |
Closing due to staleness -- feel free to reopen this PR or create a new one if you plan to keep working on this |
Resubmission of #78, which caused some compilation failures due to merge skew when it landed. @andygrove are you game to try this again?