Feat(optimizer)!: canonicalize table aliases #6369
Conversation
georgesittas
force-pushed
the
jo/canonicalize_table_aliases
branch
from
b809a2b to
5f99cef
Compare
georgesittas
force-pushed
the
jo/canonicalize_table_aliases
branch
2 times, most recently
from
5788d3a to
8060cc4
Compare
georgesittas
changed the title
georgesittas
force-pushed
the
jo/canonicalize_table_aliases
branch
from
e7e967a to
f89a683
Compare
|
This should be good to go. I'll see if I can increase the testing coverage a bit and then get it in. |
georgesittas
force-pushed
the
jo/canonicalize_table_aliases
branch
from
f89a683 to
ebb4aa2
Compare
|
Found a bug with correlated subqueries, i.e. when we have scopes w/ external columns. We need to respect lexical scoping in the canonical alias mapping probably. Going to fix and improve coverage. |
| exp.alias_(table, table.name, copy=False, table=True) | ||
| _set_alias(table, canonical_aliases, target_alias=table.name) | ||
|
|
||
| for column in local_columns: |
There was a problem hiding this comment.
Note this change vs how we previously iterated over scope.columns. If we visited external columns in this loop, we would mutate their source too early, without having had the chance to alias outer scopes' sources.
Before making this change, the test case I added in qualify_tables.sql failed due to having columns qualified with "".
| local_columns = scope.local_columns | ||
| canonical_aliases: t.Dict[str, str] = {} |
There was a problem hiding this comment.
Auto-generated alias mappings are constructed per-scope to respect lexical scoping.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This PR introduces a new
qualifyflag:canonicalize_table_aliases. This flag controls whether thequalify_tablesrule should use canonical aliases (_0,_1, ...) for all sources instead of preserving table names.