Update some analyzer rules and expression behavior to deal with tuples in a more principled way. #560
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s in a more principled way. Subquery expression nodes can now return tuples. InSubquery expression nodes can work with tuples as expected. An analyzer validation step now returns operand errors in more cases, expecting almost all expressions to return one column, but special casing certain operators and functions which support tuples. Added some TODO tests for cases where our tuple comparisons are still not behaving as we want them to. In particular, null safe vs. non-null safe comparisons and type coercion of tuple subtypes in comparisons still need work.
reltuk
commented
Sep 21, 2021
| @@ -531,22 +558,38 @@ func validateSubqueryColumns(ctx *sql.Context, a *Analyzer, n sql.Node, scope *S | |||
| return true | |||
| } | |||
|
|
|||
| outerScopeRowLen := len(schemas(n.Children())) | |||
| plan.InspectExpressionsWithNode(s.Query, func(n sql.Node, e sql.Expression) bool { | |||
There was a problem hiding this comment.
This portion of the validation never actually worked, because the if condition down below as !valid and this block never touched valid.
zachmu
approved these changes
Sep 22, 2021
enginetest/queries.go
Outdated
| Query: "SELECT 1 FROM DUAL WHERE (1, 2) in ((3, 4), (5, 6), (1, 2))", | ||
| Expected: []sql.Row{{1}}, | ||
| }, | ||
| // TODO |
| Expected: []sql.Row{{1}}, | ||
| }, | ||
| { | ||
| Query: "SELECT 1 FROM DUAL WHERE (1, 2) in ((3, 4), (5, 6), (1, 2))", |
There was a problem hiding this comment.
Missing a false case for IN for tuple literals
Also should have a couple NOT IN cases
| Expected: []sql.Row{{1}}, | ||
| }, | ||
| { | ||
| Query: "SELECT 1 FROM DUAL WHERE (null, null) <=> (select 1, 4 from dual where false)", |
There was a problem hiding this comment.
Do we expect <=> to work with nulls in tuples, e..g (null, null) <=> (select null, null)? Need a test if so, a BrokenQueries test if not
| Expected: []sql.Row{{1}}, | ||
| }, | ||
| { | ||
| Query: "SELECT (((1,2),3)) = (((1,2),3)) from dual", |
There was a problem hiding this comment.
Whoa, is this standard SQL? Tuples can nest?
sql/analyzer/validation_rules.go
Outdated
| func validateSubqueryColumns(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope) (sql.Node, error) { | ||
| func validateOperands(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope) (sql.Node, error) { | ||
| // Validate that the number of columns in an operand or a top level | ||
| // expression are as expected. The currently rules are: |
| if ok && len(s.Query.Schema()) != 1 { | ||
| valid = false | ||
| var err error | ||
| plan.Inspect(n, func(n sql.Node) bool { |
There was a problem hiding this comment.
Why not plan.InspectExpressions?
… debug analyzer output.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Subquery expression nodes can now return tuples.
InSubquery expression nodes can work with tuples as expected.
An analyzer validation step now returns operand errors in more cases, expecting
almost all expressions to return one column, but special casing certain
operators and functions which support tuples.
Added some TODO tests for cases where our tuple comparisons are still not
behaving as we want them to. In particular, null safe vs. non-null safe
comparisons and type coercion of tuple subtypes in comparisons still need work.