-
Notifications
You must be signed in to change notification settings - Fork 71
switch to pragma-controlled Postgres style relational scoping #6495
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
This commit changes the relational scoping rules to follow Postgres except for GROUP BY expressions which check column aliases before the input table (like Google SQL). This is favored as the default for SuperSQL because it doesn't change binding when a dynamic input transitions to a typed input (and thus a field that was present but unknown in the input changes the GROUP BY binding of a column alias). This behavior can be overridden now with "pragma pg=true" which says to follow Postgres semantics and favor the input table over any column alias in GROUP BY expressions. As a result, column aliases are no longer visible in WHERE clauses or SELECT expressions in line with Postgres semantics (and ANSI and Google SQL). We added some light documentation to the pragma section of the book in this PR, but more details and an explanation of the rationale for this design will be forthcoming in a future PR from the book-sql branch.
| * `1` for one-based indexing | ||
| * `pg` - controls the precedence of scoping for GROUP BY clauses in SQL operators | ||
| * `false` to follow Google SQL semantics of resolving identifiers first from column aliases then from the input table | ||
| * `true` to follow Postgres semantics of resolving identifiers first from the input table then from the column aliases |
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.
| * `true` to follow Postgres semantics of resolving identifiers first from the input table then from the column aliases | |
| * `true` to follow PostgreSQL semantics of resolving identifiers first from the input table then from the column aliases |
compiler/semantic/schema.go
Outdated
| @@ -80,7 +80,7 @@ type ( | |||
| // so latStop limits the extent of the column lookup to avoid | |||
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.
| // so latStop limits the extent of the column lookup to avoid | |
| // so lateral limits the extent of the column lookup to avoid |
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 update this comment as it was stale.
compiler/semantic/op.go
Outdated
|
|
||
| func (t *translator) mustEvalBool(in ast.Expr) (bool, bool) { | ||
| val, ok := t.mustEval(t.expr(in)) | ||
| if ok && !val.IsError() && super.TypeUnder(val.Type()) == super.TypeBool { |
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.
Don't need the error check
| if ok && !val.IsError() && super.TypeUnder(val.Type()) == super.TypeBool { | |
| if ok && super.TypeUnder(val.Type()) == super.TypeBool { |
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.
Good point. I blindly copied that pattern and it should really report a type error. Also for some of the other cases. I will address.
fix lateral column alias bug introduced in #6495 This commit fixes a bug where lateral column aliases were improperly resolved after the XXX
This commit fixes a bug where ORDER BY scope was improperly handled because the logic favored the input table over the column aliases but the materialized output table has priority over the input table once the SELECT body scope is left. This would typically be handled by first looking into the output table but we can't do this because we do expression matching on the sem-tree expressions and we can't mix and match input expressions with the output expressions. The fix is to look in the columns first (producing input table expressions) and matching those expressions to the grouped output table columns with the expression matcher.
This commit fixes a bug where ORDER BY scope was improperly handled because the logic favored the input table over the column aliases but the materialized output table has priority over the input table once the SELECT body scope is left. This would typically be handled by first looking into the output table but we can't do this because we do expression matching on the sem-tree expressions and we can't mix and match input expressions with the output expressions. The fix is to look in the columns first (producing input table expressions) and matching those expressions to the grouped output table columns with the expression matcher.
This commit changes the relational scoping rules to follow Postgres except for GROUP BY expressions which check column aliases before the input table (like Google SQL). This is favored as the default for SuperSQL because it doesn't change binding when a dynamic input transitions to a typed input (and thus a field that was present but unknown in the input changes the GROUP BY binding of a column alias).
This behavior can be overridden now with "pragma pg=true" which says to follow Postgres semantics and favor the input table over any column alias in GROUP BY expressions.
As a result, column aliases are no longer visible in WHERE clauses or SELECT expressions in line with Postgres semantics (and ANSI and Google SQL).
We added some light documentation to the pragma section of the book in this PR, but more details and an explanation of the rationale for this design will be forthcoming in a future PR from the book-sql branch.
Fixes #5974
Fixes #6410