From 9c20c16ba77e16dd804b69aa2736333b146ebfdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Taylor?= Date: Wed, 16 Oct 2024 16:28:39 +0200 Subject: [PATCH 1/2] bugfix: add HAVING columns inside derived tables (#16976) Signed-off-by: Andres Taylor --- .../operators/horizon_expanding.go | 6 ++ .../planbuilder/testdata/aggr_cases.json | 76 +++++++++++++++---- .../planbuilder/testdata/cte_cases.json | 47 ++++++++---- 3 files changed, 99 insertions(+), 30 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/horizon_expanding.go b/go/vt/vtgate/planbuilder/operators/horizon_expanding.go index 64c471ac62c..873fb575155 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_expanding.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_expanding.go @@ -83,6 +83,12 @@ func expandSelectHorizon(ctx *plancontext.PlanningContext, horizon *Horizon, sel for _, order := range horizon.Query.GetOrderBy() { qp.addDerivedColumn(ctx, order.Expr) } + sel, isSel := horizon.Query.(*sqlparser.Select) + if isSel && sel.Having != nil { + for _, pred := range sqlparser.SplitAndExpression(nil, sel.Having.Expr) { + qp.addDerivedColumn(ctx, pred) + } + } } op := createProjectionFromSelect(ctx, horizon) if qp.HasAggr { diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index 4296c72a6e6..51a40b831f3 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -571,6 +571,35 @@ ] } }, + { + "comment": "using HAVING inside a derived table still produces viable plans", + "query": "select id from (select id from user group by id having (count(user.id) = 2) limit 2 offset 0) subquery_for_limit", + "plan": { + "QueryType": "SELECT", + "Original": "select id from (select id from user group by id having (count(user.id) = 2) limit 2 offset 0) subquery_for_limit", + "Instructions": { + "OperatorType": "Limit", + "Count": "2", + "Offset": "0", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id from (select id, count(`user`.id) = 2 from `user` where 1 != 1 group by id) as subquery_for_limit where 1 != 1", + "Query": "select id from (select id, count(`user`.id) = 2 from `user` group by id having count(`user`.id) = 2) as subquery_for_limit limit 2", + "Table": "`user`" + } + ] + }, + "TablesUsed": [ + "user.user" + ] + } + }, { "comment": "sum with distinct no unique vindex", "query": "select col1, sum(distinct col2) from user group by col1", @@ -3614,25 +3643,42 @@ "QueryType": "SELECT", "Original": "select * from (select id from user having count(*) = 1) s", "Instructions": { - "OperatorType": "Filter", - "Predicate": "count(*) = 1", - "ResultColumns": 1, + "OperatorType": "SimpleProjection", + "ColumnNames": [ + "0:id" + ], + "Columns": "0", "Inputs": [ { - "OperatorType": "Aggregate", - "Variant": "Scalar", - "Aggregates": "any_value(0) AS id, sum_count_star(1) AS count(*)", + "OperatorType": "Projection", + "Expressions": [ + ":0 as id", + "count(*) = 1 as count(*) = 1" + ], "Inputs": [ { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select id, count(*) from `user` where 1 != 1", - "Query": "select id, count(*) from `user`", - "Table": "`user`" + "OperatorType": "Filter", + "Predicate": "count(*) = 1", + "Inputs": [ + { + "OperatorType": "Aggregate", + "Variant": "Scalar", + "Aggregates": "any_value(0) AS id, sum_count_star(1) AS count(*), any_value(2)", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id, count(*), 1 from `user` where 1 != 1", + "Query": "select id, count(*), 1 from `user`", + "Table": "`user`" + } + ] + } + ] } ] } diff --git a/go/vt/vtgate/planbuilder/testdata/cte_cases.json b/go/vt/vtgate/planbuilder/testdata/cte_cases.json index 51b130d25cc..517524dedcd 100644 --- a/go/vt/vtgate/planbuilder/testdata/cte_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/cte_cases.json @@ -369,25 +369,42 @@ "QueryType": "SELECT", "Original": "with s as (select id from user having count(*) = 1) select * from s", "Instructions": { - "OperatorType": "Filter", - "Predicate": "count(*) = 1", - "ResultColumns": 1, + "OperatorType": "SimpleProjection", + "ColumnNames": [ + "0:id" + ], + "Columns": "0", "Inputs": [ { - "OperatorType": "Aggregate", - "Variant": "Scalar", - "Aggregates": "any_value(0) AS id, sum_count_star(1) AS count(*)", + "OperatorType": "Projection", + "Expressions": [ + ":0 as id", + "count(*) = 1 as count(*) = 1" + ], "Inputs": [ { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select id, count(*) from `user` where 1 != 1", - "Query": "select id, count(*) from `user`", - "Table": "`user`" + "OperatorType": "Filter", + "Predicate": "count(*) = 1", + "Inputs": [ + { + "OperatorType": "Aggregate", + "Variant": "Scalar", + "Aggregates": "any_value(0) AS id, sum_count_star(1) AS count(*), any_value(2)", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id, count(*), 1 from `user` where 1 != 1", + "Query": "select id, count(*), 1 from `user`", + "Table": "`user`" + } + ] + } + ] } ] } From 26ebbb829fa2e58e26db50668cf2f9ee466e3a87 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Thu, 17 Oct 2024 09:48:41 +0200 Subject: [PATCH 2/2] bugfix: don't use derived table twice Signed-off-by: Andres Taylor --- .../planbuilder/operators/horizon_expanding.go | 1 + .../vtgate/planbuilder/testdata/aggr_cases.json | 17 ++++++++--------- .../vtgate/planbuilder/testdata/cte_cases.json | 5 ++--- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/horizon_expanding.go b/go/vt/vtgate/planbuilder/operators/horizon_expanding.go index 873fb575155..9549822274e 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_expanding.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_expanding.go @@ -300,6 +300,7 @@ outer: func createProjectionForComplexAggregation(a *Aggregator, qp *QueryProjection) Operator { p := newAliasedProjection(a) p.DT = a.DT + a.DT = nil // we don't need the derived table twice for _, expr := range qp.SelectExprs { ae, err := expr.GetAliasedExpr() if err != nil { diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index 51a40b831f3..8a703d8a620 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -590,7 +590,7 @@ "Sharded": true }, "FieldQuery": "select id from (select id, count(`user`.id) = 2 from `user` where 1 != 1 group by id) as subquery_for_limit where 1 != 1", - "Query": "select id from (select id, count(`user`.id) = 2 from `user` group by id having count(`user`.id) = 2) as subquery_for_limit limit 2", + "Query": "select id from (select id, count(`user`.id) = 2 from `user` group by id having count(`user`.id) = 2) as subquery_for_limit limit :__upper_limit", "Table": "`user`" } ] @@ -3644,10 +3644,9 @@ "Original": "select * from (select id from user having count(*) = 1) s", "Instructions": { "OperatorType": "SimpleProjection", - "ColumnNames": [ - "0:id" + "Columns": [ + 0 ], - "Columns": "0", "Inputs": [ { "OperatorType": "Projection", @@ -6594,11 +6593,11 @@ { "OperatorType": "SimpleProjection", "Columns": [ - 2, - 0, - 1, - 3 - ], + 2, + 0, + 1, + 3 + ], "Inputs": [ { "OperatorType": "Sort", diff --git a/go/vt/vtgate/planbuilder/testdata/cte_cases.json b/go/vt/vtgate/planbuilder/testdata/cte_cases.json index 517524dedcd..2fd8d573fe0 100644 --- a/go/vt/vtgate/planbuilder/testdata/cte_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/cte_cases.json @@ -370,10 +370,9 @@ "Original": "with s as (select id from user having count(*) = 1) select * from s", "Instructions": { "OperatorType": "SimpleProjection", - "ColumnNames": [ - "0:id" + "Columns": [ + 0 ], - "Columns": "0", "Inputs": [ { "OperatorType": "Projection",