-
Notifications
You must be signed in to change notification settings - Fork 3
[BSE-4250] Infrastructure for converting Relational to SQL #65
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One big note, but I'll leave it to your judgement on how to address it.
input=build_simple_scan(), | ||
ordered_columns=[("b", make_relational_column_reference("b"))], | ||
), | ||
"SELECT b FROM table", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳 🥳 🥳 🥳 🥳 🥳 🥳
pydough/sqlglot/relational_to_sql.py
Outdated
# Cache a visitor for the module | ||
_visitor: SQLGlotRelationalVisitor = SQLGlotRelationalVisitor() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I advise against having a single defined visitor. When we get around to redefining how function translation works, the binding setup will likely be dialect-dependant, and each visitor will accordingly need its own bindings based on the dialect (or even on any functions the user defines in context). For the sake of flexibility, I advise we instead define some kind of datastructure that can host multiple visitors, at minimum broken up by dialect. For now, we'll only ever create & use one of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I can move it out here since this is an over optimization.
Adds the basic support for converting relational to SQL. This is a simple test for the most basic functionality + API. The followup PR will add tests for every type of operator.