-
Notifications
You must be signed in to change notification settings - Fork 94
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
Update SqlTableAliasSimplifier
to handle CTEs
#1516
Conversation
mf_test_configuration: MetricFlowTestConfiguration, | ||
sql_plan_renderer: DefaultSqlQueryPlanRenderer, | ||
) -> None: | ||
"""Tests that table aliases are removed when not needed in CTEs.""" |
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.
Should we also have a test to demonstrate that table aliases are not removed if they are needed in CTEs?
I'm assuming that would only happen if there was a join within the CTE statement. Not sure if that case will ever occur with the current CTE implementation.
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.
Can't hurt - added a test case.
Similar to the updates for the other optimizers in #1503 and #1504, this PR implements the CTE case for the
SqlTableAliasSimplifier