Skip to content
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.

Sign up for GitHub

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

Copy expression types to avoid weight_strings and derived tables #15069

Merged
merged 3 commits into from
Feb 2, 2024
Merged

Copy expression types to avoid weight_strings and derived tables #15069

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1827e5
copying the types, some Union operators do not need to be transformed…
wangweicugw Jan 29, 2024
f35b917
fix the types of columns on both sides of the union are inconsistent
wangweicugw Jan 31, 2024
f1b01ea
remove else
wangweicugw Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions go/test/endtoend/vtgate/queries/union/union_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,21 @@ func TestUnionDistinct(t *testing.T) {
t.Skip()
mcmp.AssertMatches("select 1 from dual where 1 IN (select 1 as col union select 2)", "[[INT64(1)]]")
})
if utils.BinaryIsAtLeastAtVersion(19, "vtgate") {
mcmp.AssertMatches(`SELECT 1 from t1 UNION SELECT 2 from t1`, `[[INT64(1)] [INT64(2)]]`)
mcmp.AssertMatches(`SELECT 5 from t1 UNION SELECT 6 from t1`, `[[INT64(5)] [INT64(6)]]`)
mcmp.AssertMatchesNoOrder(`SELECT id1 from t1 UNION SELECT id2 from t1`, `[[INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)]]`)
mcmp.AssertMatchesNoOrder(`SELECT 1 from t1 UNION SELECT id2 from t1`, `[[INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)]]`)
mcmp.AssertMatchesNoOrder(`SELECT 5 from t1 UNION SELECT id2 from t1`, `[[INT64(5)] [INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)]]`)
mcmp.AssertMatchesNoOrder(`SELECT id1 from t1 UNION SELECT 2 from t1`, `[[INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)]]`)
mcmp.AssertMatchesNoOrder(`SELECT id1 from t1 UNION SELECT 5 from t1`, `[[INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)] [INT64(5)]]`)
mcmp.AssertMatchesNoOrder(`select 3 from t1 union select DAY("2024-01-31") from t1`, `[[INT64(3)] [INT64(31)]]`)
mcmp.AssertMatchesNoOrder(`select DAY("2024-01-31") from t1 union select 3 from t1`, `[[INT64(31)] [INT64(3)]]`)
mcmp.AssertMatchesNoOrder(`select DAY("2024-01-31") from t1 union select id1 from t1`, `[[INT64(31)] [INT64(1)] [INT64(2)] [INT64(3)] [INT64(4)]]`)
mcmp.Exec(`select 3 from t1 union select curdate() from t1`)
mcmp.Exec(`select curdate() from t1 union select 3 from t1`)
mcmp.Exec(`select curdate() from t1 union select id1 from t1`)
}
})

}
Expand Down Expand Up @@ -130,11 +145,4 @@ func TestUnion(t *testing.T) {
if utils.BinaryIsAtLeastAtVersion(19, "vtgate") {
mcmp.AssertMatches(`(SELECT id2,'a' from t1 where id1 = 1) union (SELECT 'a',id2 from t1 where id1 = 2)`, `[[VARCHAR("1") VARCHAR("a")] [VARCHAR("a") VARCHAR("2")]]`)
}
mcmp.AssertMatches(`SELECT 1 from t1 UNION SELECT 2 from t1`, `[[INT64(1)] [INT64(2)]]`)
mcmp.AssertMatches(`SELECT 4 from t1 UNION SELECT 3 from t1`, `[[INT64(4)] [INT64(3)]]`)
mcmp.AssertMatches(`SELECT id1 from t1 UNION SELECT id2 from t1`, `[[INT64(1)] [INT64(2)]]`)
mcmp.AssertMatches(`SELECT 1 from t1 UNION SELECT id2 from t1`, `[[INT64(1)] [INT64(2)]]`)
mcmp.AssertMatches(`SELECT 3 from t1 UNION SELECT id2 from t1`, `[[INT64(3)] [INT64(1)] [INT64(2)]]`)
mcmp.AssertMatches(`SELECT id1 from t1 UNION SELECT 2 from t1`, `[[INT64(1)] [INT64(2)]]`)
mcmp.AssertMatches(`SELECT id1 from t1 UNION SELECT 3 from t1`, `[[INT64(1)] [INT64(2)] [INT64(3)]]`)
}
15 changes: 13 additions & 2 deletions go/vt/vtgate/planbuilder/operators/union_merging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ limitations under the License.
package operators

import (
"vitess.io/vitess/go/mysql/collations"
"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vtgate/engine"
"vitess.io/vitess/go/vt/vtgate/evalengine"
"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
)

Expand Down Expand Up @@ -199,8 +202,16 @@ func createMergedUnion(
continue
}
deps = deps.Merge(ctx.SemTable.RecursiveDeps(rae.Expr))
ctx.SemTable.CopySemanticInfo(rae.Expr, col)
ctx.SemTable.CopySemanticInfo(lae.Expr, col)
rt, foundR := ctx.SemTable.TypeForExpr(rae.Expr)
lt, foundL := ctx.SemTable.TypeForExpr(lae.Expr)
if foundR && foundL {
types := []sqltypes.Type{rt.Type(), lt.Type()}
t := evalengine.AggregateTypes(types)
ctx.SemTable.ExprTypes[col] = evalengine.NewType(t, collations.Unknown)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@systay Shouldn't this have used AggregateEvalTypes so that the collation is also known and not collations.Unknown?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, #15122 answers my question here.

} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should have the else clause here. It's not correct to use either of the two types as the common type. Like my example showed, you can have a situation where one side is an int, the other side is a datetime, but the common type is varchar.

If we don't have both types available, I think we need to use the weight_string function to be sure we can compare values correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now. There shouldn't be an "else" processing logic.

If it is a syntax tree node for the now() function, we can save its corresponding evalengine.Type in ExprTypes during the analysis phase in the analyzer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we could/should update typer.go and add more expressions there. Or, even better, start using the excellent typing logic we have in the evalengine. One of these days...

ctx.SemTable.CopySemanticInfo(rae.Expr, col)
ctx.SemTable.CopySemanticInfo(lae.Expr, col)
}
ctx.SemTable.Recursive[col] = deps
}

Expand Down
91 changes: 91 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/union_cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1622,5 +1622,96 @@
"user.user"
]
}
},
{
"comment": "Select literals from table union Select now() from table",
"query": "select 3 from user union select now() from user",
"plan": {
"QueryType": "SELECT",
"Original": "select 3 from user union select now() from user",
"Instructions": {
"OperatorType": "Distinct",
"Collations": [
"0"
],
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select 3 from `user` where 1 != 1 union select now() from `user` where 1 != 1",
"Query": "select 3 from `user` union select now() from `user`",
"Table": "`user`"
}
]
},
"TablesUsed": [
"user.user"
]
}
},
{
"comment": "Select now() from table union Select literals from table",
"query": "select now() from user union select 3 from user",
"plan": {
"QueryType": "SELECT",
"Original": "select now() from user union select 3 from user",
"Instructions": {
"OperatorType": "Distinct",
"Collations": [
"0"
],
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select now() from `user` where 1 != 1 union select 3 from `user` where 1 != 1",
"Query": "select now() from `user` union select 3 from `user`",
"Table": "`user`"
}
]
},
"TablesUsed": [
"user.user"
]
}
},
{
"comment": "Select now() from table union Select column from table",
"query": "select now() from user union select id from user",
"plan": {
"QueryType": "SELECT",
"Original": "select now() from user union select id from user",
"Instructions": {
"OperatorType": "Distinct",
"Collations": [
"(0:1)"
],
"ResultColumns": 1,
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select `now()`, weight_string(`now()`) from (select now() from `user` where 1 != 1 union select id from `user` where 1 != 1) as dt where 1 != 1",
"Query": "select `now()`, weight_string(`now()`) from (select now() from `user` union select id from `user`) as dt",
"Table": "`user`"
}
]
},
"TablesUsed": [
"user.user"
]
}
}
]