-
Notifications
You must be signed in to change notification settings - Fork 769
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
Refactor(optimizer): improve handling of DDL optimization #1972
Conversation
@@ -990,7 +990,7 @@ class Uncache(Expression): | |||
arg_types = {"this": True, "exists": False} | |||
|
|||
|
|||
class TableChange(Expression): | |||
class DDL(Expression): |
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 didn't have UPDATE
inherit from this on purpose because it doesn't have a Subqueryable
expression like CREATE
and INSERT
, so it'd need more effort to qualify / optimize its SET .. FROM ..
syntax. Perhaps we could tackle this in a future PR.
with_ = scope.expression.args.get("with") | ||
if with_ and with_.recursive: |
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.
In WITH ... CREATE ... AS SELECT ...
the CTEs are not attached to the SELECT
, so args["with"]
would raise a KeyError
here.
query_scope = scope.branch( | ||
scope.expression.expression, scope_type=ScopeType.DERIVED_TABLE, chain_sources=scope.sources | ||
) |
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.
Picked DERIVED_TABLE
here because ROOT
is already taken for the DDL statement itself. Might not be the cleanest approach but I think it's fine for now?
Regarding the comment I left here #1949 (comment), it seems like this is a known "issue" based on: sqlglot/tests/fixtures/optimizer/qualify_tables.sql Lines 17 to 20 in 9d67283
I assume this is because when we do |
fe30934
to
719bea2
Compare
cc @gtoonstra
Maybe the name
DDL
isn't the best for this, given thatINSERT
is actually in the DML category..