Skip to content

Commit

Permalink
Fix aggregation on outer joins (vitessio#12298)
Browse files Browse the repository at this point in the history
* fix: aggregation coming from right hand side of left join check for null

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

* test: added a sum test

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

* addressed review comments

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

---------

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
  • Loading branch information
harshit-gangal committed Feb 10, 2023
1 parent 36914bf commit 062ffb7
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 18 deletions.
14 changes: 14 additions & 0 deletions go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,17 @@ func TestAggregateRandom(t *testing.T) {
utils.Exec(t, mcmp.MySQLConn, "set sql_mode=''")
mcmp.AssertMatches("SELECT /*vt+ PLANNER=gen4 */ t1.shardKey, t1.name, count(t2.id) FROM t1 JOIN t2 ON t1.value != t2.shardKey GROUP BY t1.t1_id", `[[INT64(1) VARCHAR("name 1") INT64(2)] [INT64(2) VARCHAR("name 2") INT64(2)]]`)
}

// TestAggregateLeftJoin tests that aggregates work with left joins and does not ignore the count when column value does not match the right side table.
func TestAggregateLeftJoin(t *testing.T) {
mcmp, closer := start(t)
defer closer()

mcmp.Exec("insert into t1(t1_id, name, value, shardKey) values (11, 'r', 'r', 1), (3, 'r', 'r', 0)")
mcmp.Exec("insert into t2(id, shardKey) values (11, 1)")

mcmp.AssertMatchesNoOrder("SELECT t1.shardkey FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[INT64(1)] [INT64(0)]]`)
mcmp.AssertMatches("SELECT count(t1.shardkey) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[INT64(2)]]`)
mcmp.AssertMatches("SELECT count(*) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[INT64(2)]]`)
mcmp.AssertMatches("SELECT sum(t1.shardkey) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[DECIMAL(1)]]`)
}
8 changes: 7 additions & 1 deletion go/vt/vtgate/planbuilder/horizon_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,13 @@ func generateAggregateParams(aggrs []abstract.Aggr, aggrParamOffsets [][]offsets
aggrExpr = &sqlparser.BinaryExpr{
Operator: sqlparser.MultOp,
Left: aggrExpr,
Right: curr,
Right: &sqlparser.FuncExpr{
Name: sqlparser.NewColIdent("coalesce"),
Exprs: sqlparser.SelectExprs{
&sqlparser.AliasedExpr{Expr: curr},
&sqlparser.AliasedExpr{Expr: sqlparser.NewIntLiteral("1")},
},
},
}
}
}
Expand Down
74 changes: 65 additions & 9 deletions go/vt/vtgate/planbuilder/testdata/aggr_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -3432,7 +3432,7 @@
{
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] * [COLUMN 1] as count(*)"
"[COLUMN 0] * COALESCE([COLUMN 1], INT64(1)) as count(*)"
],
"Inputs": [
{
Expand Down Expand Up @@ -3544,7 +3544,7 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as a",
"[COLUMN 2] * [COLUMN 3] as count(*)",
"[COLUMN 2] * COALESCE([COLUMN 3], INT64(1)) as count(*)",
"[COLUMN 1]"
],
"Inputs": [
Expand Down Expand Up @@ -3603,7 +3603,7 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as a",
"[COLUMN 2] * [COLUMN 3] as count(user_extra.a)",
"[COLUMN 2] * COALESCE([COLUMN 3], INT64(1)) as count(user_extra.a)",
"[COLUMN 1]"
],
"Inputs": [
Expand Down Expand Up @@ -3661,8 +3661,8 @@
{
"OperatorType": "Projection",
"Expressions": [
"([COLUMN 2] * [COLUMN 3]) * [COLUMN 4] as count(u.textcol1)",
"([COLUMN 5] * [COLUMN 6]) * [COLUMN 7] as count(ue.foo)",
"([COLUMN 2] * COALESCE([COLUMN 3], INT64(1))) * COALESCE([COLUMN 4], INT64(1)) as count(u.textcol1)",
"([COLUMN 5] * COALESCE([COLUMN 6], INT64(1))) * COALESCE([COLUMN 7], INT64(1)) as count(ue.foo)",
"[COLUMN 0] as bar",
"[COLUMN 1]"
],
Expand Down Expand Up @@ -4224,7 +4224,7 @@
{
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 2] * [COLUMN 3] as sum(col)"
"[COLUMN 2] * COALESCE([COLUMN 3], INT64(1)) as sum(col)"
],
"Inputs": [
{
Expand Down Expand Up @@ -4466,7 +4466,7 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as id",
"[COLUMN 2] * [COLUMN 3] as count(u.`name`)",
"[COLUMN 2] * COALESCE([COLUMN 3], INT64(1)) as count(u.`name`)",
"[COLUMN 1]"
],
"Inputs": [
Expand Down Expand Up @@ -4584,7 +4584,7 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as id",
"[COLUMN 2] * [COLUMN 3] as count(*)",
"[COLUMN 2] * COALESCE([COLUMN 3], INT64(1)) as count(*)",
"[COLUMN 1]"
],
"Inputs": [
Expand Down Expand Up @@ -5164,7 +5164,7 @@
"Expressions": [
"[COLUMN 0] as id",
"[COLUMN 2] as name",
"[COLUMN 3] * [COLUMN 4] as count(m.predef1)",
"[COLUMN 3] * COALESCE([COLUMN 4], INT64(1)) as count(m.predef1)",
"[COLUMN 1]"
],
"Inputs": [
Expand Down Expand Up @@ -5217,5 +5217,61 @@
]
}
}
},
{
"comment": "Aggregation in a left join query",
"query": "select count (u.id) from user u left join user_extra ue on u.col = ue.col",
"plan": {
"QueryType": "SELECT",
"Original": "select count (u.id) from user u left join user_extra ue on u.col = ue.col",
"Instructions": {
"OperatorType": "Aggregate",
"Variant": "Scalar",
"Aggregates": "sum_count(0) AS count(u.id)",
"Inputs": [
{
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] * COALESCE([COLUMN 1], INT64(1)) as count(u.id)"
],
"Inputs": [
{
"OperatorType": "Join",
"Variant": "LeftJoin",
"JoinColumnIndexes": "L:1,R:1",
"JoinVars": {
"u_col": 0
},
"TableName": "`user`_user_extra",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select u.col, count(u.id) from `user` as u where 1 != 1 group by u.col",
"Query": "select u.col, count(u.id) from `user` as u group by u.col",
"Table": "`user`"
},
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select 1, count(*) from user_extra as ue where 1 != 1 group by 1",
"Query": "select 1, count(*) from user_extra as ue where ue.col = :u_col group by 1",
"Table": "user_extra"
}
]
}
]
}
]
}
}
}
]
16 changes: 8 additions & 8 deletions go/vt/vtgate/planbuilder/testdata/tpch_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as l_orderkey",
"([COLUMN 6] * [COLUMN 7]) * [COLUMN 8] as revenue",
"([COLUMN 6] * COALESCE([COLUMN 7], INT64(1))) * COALESCE([COLUMN 8], INT64(1)) as revenue",
"[COLUMN 1] as o_orderdate",
"[COLUMN 2] as o_shippriority",
"[COLUMN 5]",
Expand Down Expand Up @@ -213,7 +213,7 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as n_name",
"(((([COLUMN 2] * [COLUMN 3]) * [COLUMN 4]) * [COLUMN 5]) * [COLUMN 6]) * [COLUMN 7] as revenue",
"(((([COLUMN 2] * COALESCE([COLUMN 3], INT64(1))) * COALESCE([COLUMN 4], INT64(1))) * COALESCE([COLUMN 5], INT64(1))) * COALESCE([COLUMN 6], INT64(1))) * COALESCE([COLUMN 7], INT64(1)) as revenue",
"[COLUMN 1]"
],
"Inputs": [
Expand Down Expand Up @@ -442,7 +442,7 @@
"[COLUMN 4] as supp_nation",
"[COLUMN 5] as cust_nation",
"[COLUMN 6] as l_year",
"(((([COLUMN 10] * [COLUMN 11]) * [COLUMN 12]) * [COLUMN 13]) * [COLUMN 14]) * [COLUMN 15] as revenue",
"(((([COLUMN 10] * COALESCE([COLUMN 11], INT64(1))) * COALESCE([COLUMN 12], INT64(1))) * COALESCE([COLUMN 13], INT64(1))) * COALESCE([COLUMN 14], INT64(1))) * COALESCE([COLUMN 15], INT64(1)) as revenue",
"[COLUMN 9]",
"[COLUMN 8]",
"[COLUMN 7]"
Expand Down Expand Up @@ -642,7 +642,7 @@
"Expressions": [
"[COLUMN 0] as c_custkey",
"[COLUMN 1] as c_name",
"(([COLUMN 14] * [COLUMN 15]) * [COLUMN 16]) * [COLUMN 17] as revenue",
"(([COLUMN 14] * COALESCE([COLUMN 15], INT64(1))) * COALESCE([COLUMN 16], INT64(1))) * COALESCE([COLUMN 17], INT64(1)) as revenue",
"[COLUMN 2] as c_acctbal",
"[COLUMN 4] as n_name",
"[COLUMN 5] as c_address",
Expand Down Expand Up @@ -787,8 +787,8 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as l_shipmode",
"[COLUMN 2] * [COLUMN 3] as high_line_count",
"[COLUMN 4] * [COLUMN 5] as low_line_count",
"[COLUMN 2] * COALESCE([COLUMN 3], INT64(1)) as high_line_count",
"[COLUMN 4] * COALESCE([COLUMN 5], INT64(1)) as low_line_count",
"[COLUMN 1]"
],
"Inputs": [
Expand Down Expand Up @@ -1006,7 +1006,7 @@
{
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] * [COLUMN 1] as revenue"
"[COLUMN 0] * COALESCE([COLUMN 1], INT64(1)) as revenue"
],
"Inputs": [
{
Expand Down Expand Up @@ -1084,7 +1084,7 @@
"OperatorType": "Projection",
"Expressions": [
"[COLUMN 0] as s_name",
"(([COLUMN 2] * [COLUMN 3]) * [COLUMN 4]) * [COLUMN 5] as numwait",
"(([COLUMN 2] * COALESCE([COLUMN 3], INT64(1))) * COALESCE([COLUMN 4], INT64(1))) * COALESCE([COLUMN 5], INT64(1)) as numwait",
"[COLUMN 1]"
],
"Inputs": [
Expand Down

0 comments on commit 062ffb7

Please sign in to comment.