-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support uses of BACK that cause correlated references: SQLGlot conversion #234
Support uses of BACK that cause correlated references: SQLGlot conversion #234
Conversation
Co-authored-by: Hadia Ahmed <[email protected]>
Revision 2 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 3 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 4 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 5 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 6 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 7 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 8 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 9 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 10 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 11 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 12 Co-authored-by: Hadia Ahmed <[email protected]>
Revision 13 Co-authored-by: Hadia Ahmed <[email protected]>
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.
no major things to share here.
seen_cols.add(expr) | ||
else: | ||
raise ValueError( | ||
f"Unsupported expression type for column merging: {new_column.__class__.__name__}" |
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.
are you able to provide column name here?
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.
A column name doesn't necessarily exist in this case (depends on what type new_column
is and this is the catch-all for all unsupported types). This case should never occur by construction of the _is_mergeable_column
API which prevents anything except these cases.
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.
LGTM.
Child PR of #269 that is part of addressing #141. Adds the SQLGlot conversion step. With these changes, all SQLGlot correlation queries are functional except for:
These remainders need to be handled via the decorrelation cases in the comments of #141. Specifically: