From 3b60479df7d193b0dcd24a2dbd0de522e0ff5b17 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 10 Jul 2024 09:47:15 +0200 Subject: [PATCH 1/5] backport of https://github.com/vitessio/vitess/pull/16353 Signed-off-by: Andres Taylor --- .../operators/horizon_expanding.go | 9 +++- .../planbuilder/operators/queryprojection.go | 17 +++++++ .../planbuilder/testdata/aggr_cases.json | 45 +++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/operators/horizon_expanding.go b/go/vt/vtgate/planbuilder/operators/horizon_expanding.go index 66190a4eeef..0a95cd5e5e7 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_expanding.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_expanding.go @@ -75,9 +75,16 @@ func expandUnionHorizon(ctx *plancontext.PlanningContext, horizon *Horizon, unio } func expandSelectHorizon(ctx *plancontext.PlanningContext, horizon *Horizon, sel *sqlparser.Select) (Operator, *ApplyResult) { - op := createProjectionFromSelect(ctx, horizon) qp := horizon.getQP(ctx) var extracted []string + if horizon.IsDerived() { + // if we are dealing with a derived table, we need to make sure that the ordering columns + // are available outside the derived table + for _, order := range horizon.Query.GetOrderBy() { + qp.addColumn(ctx, order.Expr) + } + } + op := createProjectionFromSelect(ctx, horizon) if qp.HasAggr { extracted = append(extracted, "Aggregation") } else { diff --git a/go/vt/vtgate/planbuilder/operators/queryprojection.go b/go/vt/vtgate/planbuilder/operators/queryprojection.go index d34422a8d4d..3bf22364cbd 100644 --- a/go/vt/vtgate/planbuilder/operators/queryprojection.go +++ b/go/vt/vtgate/planbuilder/operators/queryprojection.go @@ -743,6 +743,23 @@ func (qp *QueryProjection) useGroupingOverDistinct(ctx *plancontext.PlanningCont return true } +// addColumn adds a column to the QueryProjection if it is not already present +func (qp *QueryProjection) addColumn(ctx *plancontext.PlanningContext, expr sqlparser.Expr) { + for _, selectExpr := range qp.SelectExprs { + getExpr, err := selectExpr.GetExpr() + if err != nil { + continue + } + if ctx.SemTable.EqualsExprWithDeps(getExpr, expr) { + return + } + } + qp.SelectExprs = append(qp.SelectExprs, SelectExpr{ + Col: aeWrap(expr), + Aggr: containsAggr(expr), + }) +} + func checkForInvalidGroupingExpressions(expr sqlparser.Expr) { _ = sqlparser.Walk(func(node sqlparser.SQLNode) (bool, error) { if _, isAggregate := node.(sqlparser.AggrFunc); isAggregate { diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index f7e556956e3..961f81769a8 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -6696,6 +6696,51 @@ ] } }, + { + "comment": "Aggregation over a ORDER BY/LIMIT inside a derived table", + "query": "SELECT COUNT(*) FROM (SELECT 1 AS one FROM `user` WHERE `user`.`is_not_deleted` = true ORDER BY id DESC LIMIT 25 OFFSET 0) subquery_for_count", + "plan": { + "QueryType": "SELECT", + "Original": "SELECT COUNT(*) FROM (SELECT 1 AS one FROM `user` WHERE `user`.`is_not_deleted` = true ORDER BY id DESC LIMIT 25 OFFSET 0) subquery_for_count", + "Instructions": { + "OperatorType": "Aggregate", + "Variant": "Scalar", + "Aggregates": "count_star(0) AS count(*)", + "Inputs": [ + { + "OperatorType": "SimpleProjection", + "Columns": [ + 2 + ], + "Inputs": [ + { + "OperatorType": "Limit", + "Count": "25", + "Offset": "0", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select subquery_for_count.one, subquery_for_count.id, 1, weight_string(subquery_for_count.id) from (select 1 as one, id from `user` where 1 != 1) as subquery_for_count where 1 != 1", + "OrderBy": "(1|3) DESC", + "Query": "select subquery_for_count.one, subquery_for_count.id, 1, weight_string(subquery_for_count.id) from (select 1 as one, id from `user` where `user`.is_not_deleted = true) as subquery_for_count order by id desc limit :__upper_limit", + "Table": "`user`" + } + ] + } + ] + } + ] + }, + "TablesUsed": [ + "user.user" + ] + } + }, { "comment": "sharded subquery inside group_concat multi-column aggregation function on a sharded table on same vindex value", "query": "select max((select group_concat(col1, col2) from user where id = 1)) from user where id = 1", From de4a805e5002fbc3c2a44a31b9e5ae9f6ad698ae Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 10 Jul 2024 09:47:43 +0200 Subject: [PATCH 2/5] empty commit to trigger ci Signed-off-by: Andres Taylor From bce13f7887dcd999524454b1856f8a95152c52b1 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 10 Jul 2024 09:54:16 +0200 Subject: [PATCH 3/5] add e2e test Signed-off-by: Andres Taylor --- .../endtoend/vtgate/queries/aggregation/aggregation_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go index 9a10b40db15..cb91a401cb7 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go +++ b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go @@ -433,7 +433,8 @@ func TestOrderByCount(t *testing.T) { mcmp.Exec("insert into t9(id1, id2, id3) values(1, '1', '1'), (2, '2', '2'), (3, '2', '2'), (4, '3', '3'), (5, '3', '3'), (6, '3', '3')") - mcmp.AssertMatches("SELECT t9.id2 FROM t9 GROUP BY t9.id2 ORDER BY COUNT(t9.id2) DESC", `[[VARCHAR("3")] [VARCHAR("2")] [VARCHAR("1")]]`) + mcmp.Exec("SELECT t9.id2 FROM t9 GROUP BY t9.id2 ORDER BY COUNT(t9.id2) DESC") + mcmp.Exec("select COUNT(*) from (select 1 as one FROM t9 WHERE id3 = 3 ORDER BY id1 DESC LIMIT 3 OFFSET 0) subquery_for_count") } func TestAggregateAnyValue(t *testing.T) { From 51e20395590595724f8b30cc5fff9b3a42d531cd Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 10 Jul 2024 09:58:57 +0200 Subject: [PATCH 4/5] restrict test to v19 Signed-off-by: Andres Taylor --- .../endtoend/vtgate/queries/aggregation/aggregation_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go index cb91a401cb7..d638f2f53b4 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go +++ b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go @@ -434,7 +434,9 @@ func TestOrderByCount(t *testing.T) { mcmp.Exec("insert into t9(id1, id2, id3) values(1, '1', '1'), (2, '2', '2'), (3, '2', '2'), (4, '3', '3'), (5, '3', '3'), (6, '3', '3')") mcmp.Exec("SELECT t9.id2 FROM t9 GROUP BY t9.id2 ORDER BY COUNT(t9.id2) DESC") - mcmp.Exec("select COUNT(*) from (select 1 as one FROM t9 WHERE id3 = 3 ORDER BY id1 DESC LIMIT 3 OFFSET 0) subquery_for_count") + if utils.BinaryIsAtLeastAtVersion(19, "vtgate") { + mcmp.Exec("select COUNT(*) from (select 1 as one FROM t9 WHERE id3 = 3 ORDER BY id1 DESC LIMIT 3 OFFSET 0) subquery_for_count") + } } func TestAggregateAnyValue(t *testing.T) { From d3e7b211280fe246c58dc0d01891427aefebc1f3 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 10 Jul 2024 11:39:17 +0200 Subject: [PATCH 5/5] pin to specific version Signed-off-by: Andres Taylor --- .../endtoend/vtgate/queries/aggregation/aggregation_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go index d638f2f53b4..7a08dffda19 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go +++ b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go @@ -434,7 +434,9 @@ func TestOrderByCount(t *testing.T) { mcmp.Exec("insert into t9(id1, id2, id3) values(1, '1', '1'), (2, '2', '2'), (3, '2', '2'), (4, '3', '3'), (5, '3', '3'), (6, '3', '3')") mcmp.Exec("SELECT t9.id2 FROM t9 GROUP BY t9.id2 ORDER BY COUNT(t9.id2) DESC") - if utils.BinaryIsAtLeastAtVersion(19, "vtgate") { + version, err := cluster.GetMajorVersion("vtgate") + require.NoError(t, err) + if version == 19 { mcmp.Exec("select COUNT(*) from (select 1 as one FROM t9 WHERE id3 = 3 ORDER BY id1 DESC LIMIT 3 OFFSET 0) subquery_for_count") } }