From 02a54a636fae13552540400287c4817f063cc249 Mon Sep 17 00:00:00 2001 From: FlorentP <35779988+frouioui@users.noreply.github.com> Date: Thu, 13 Oct 2022 17:15:21 +0200 Subject: [PATCH] Plan order by `COUNT(X)` (#11420) * Plan order by Count() Signed-off-by: Florent Poinsard * Clean up the new aggregation E2E test Signed-off-by: Florent Poinsard * Push more order by needs to the select list Signed-off-by: Florent Poinsard * Remove unrequired code in TestOrderByCount Signed-off-by: Florent Poinsard * remove unwanted directory Signed-off-by: Florent Poinsard Signed-off-by: Florent Poinsard --- .../queries/aggregation/aggregation_test.go | 11 ++++- .../vtgate/queries/aggregation/main_test.go | 4 +- .../vtgate/queries/aggregation/schema.sql | 1 + .../planbuilder/abstract/queryprojection.go | 38 +----------------- .../planbuilder/testdata/aggr_cases.json | 40 +++++++++++++++++++ 5 files changed, 54 insertions(+), 40 deletions(-) diff --git a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go index dfcfcc0c426..df95b679a1f 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go +++ b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go @@ -33,7 +33,7 @@ func start(t *testing.T) (utils.MySQLCompare, func()) { deleteAll := func() { _, _ = utils.ExecAllowError(t, mcmp.VtConn, "set workload = oltp") - tables := []string{"aggr_test", "t3", "t7_xxhash", "aggr_test_dates", "t7_xxhash_idx", "t1", "t2"} + tables := []string{"t9", "aggr_test", "t3", "t7_xxhash", "aggr_test_dates", "t7_xxhash_idx", "t1", "t2"} for _, table := range tables { _, _ = mcmp.ExecAndIgnore("delete from " + table) } @@ -365,3 +365,12 @@ func TestEmptyTableAggr(t *testing.T) { } } + +func TestOrderByCount(t *testing.T) { + mcmp, closer := start(t) + defer closer() + + 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 /*vt+ PLANNER=gen4 */ t9.id2 FROM t9 GROUP BY t9.id2 ORDER BY COUNT(t9.id2) DESC", `[[VARCHAR("3")] [VARCHAR("2")] [VARCHAR("1")]]`) +} diff --git a/go/test/endtoend/vtgate/queries/aggregation/main_test.go b/go/test/endtoend/vtgate/queries/aggregation/main_test.go index 65cf3a0343b..a859002f44a 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/main_test.go +++ b/go/test/endtoend/vtgate/queries/aggregation/main_test.go @@ -33,8 +33,8 @@ var ( clusterInstance *cluster.LocalProcessCluster vtParams mysql.ConnParams mysqlParams mysql.ConnParams - keyspaceName = "ks_union" - cell = "test_union" + keyspaceName = "ks_aggr" + cell = "test_aggr" //go:embed schema.sql schemaSQL string diff --git a/go/test/endtoend/vtgate/queries/aggregation/schema.sql b/go/test/endtoend/vtgate/queries/aggregation/schema.sql index 944c3783048..a6ce793b4e7 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/schema.sql +++ b/go/test/endtoend/vtgate/queries/aggregation/schema.sql @@ -69,3 +69,4 @@ CREATE TABLE t2 ( shardKey bigint, PRIMARY KEY (id) ) ENGINE InnoDB; + diff --git a/go/vt/vtgate/planbuilder/abstract/queryprojection.go b/go/vt/vtgate/planbuilder/abstract/queryprojection.go index 14572b117b2..f3acdd3693c 100644 --- a/go/vt/vtgate/planbuilder/abstract/queryprojection.go +++ b/go/vt/vtgate/planbuilder/abstract/queryprojection.go @@ -314,39 +314,6 @@ func checkForInvalidAggregations(exp *sqlparser.AliasedExpr) error { }, exp.Expr) } -func (qp *QueryProjection) getNonAggrExprNotMatchingGroupByExprs() sqlparser.SelectExpr { - for _, expr := range qp.SelectExprs { - if expr.Aggr { - continue - } - if !qp.isExprInGroupByExprs(expr) { - return expr.Col - } - } - for _, order := range qp.OrderExprs { - if !qp.isOrderByExprInGroupBy(order) { - return &sqlparser.AliasedExpr{ - Expr: order.Inner.Expr, - } - } - } - return nil -} - -func (qp *QueryProjection) isOrderByExprInGroupBy(order OrderBy) bool { - // ORDER BY NULL or Aggregation functions need not be present in group by - _, isAggregate := order.WeightStrExpr.(sqlparser.AggrFunc) - if sqlparser.IsNull(order.Inner.Expr) || isAggregate { - return true - } - for _, groupByExpr := range qp.groupByExprs { - if sqlparser.EqualsExpr(groupByExpr.WeightStrExpr, order.WeightStrExpr) { - return true - } - } - return false -} - func (qp *QueryProjection) isExprInGroupByExprs(expr SelectExpr) bool { for _, groupByExpr := range qp.groupByExprs { exp, err := expr.GetExpr() @@ -457,10 +424,7 @@ func (qp *QueryProjection) NeedsDistinct() bool { func (qp *QueryProjection) AggregationExpressions() (out []Aggr, err error) { orderBy: for _, orderExpr := range qp.OrderExprs { - if qp.isOrderByExprInGroupBy(orderExpr) { - continue orderBy - } - orderExpr := orderExpr.Inner.Expr + orderExpr := orderExpr.WeightStrExpr for _, expr := range qp.SelectExprs { col, ok := expr.Col.(*sqlparser.AliasedExpr) if !ok { diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index 1fc9db15e72..45a6cb7238a 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -4779,5 +4779,45 @@ "user.user" ] } + }, + { + "comment": "Group By X Order By X", + "query": "SELECT user.intcol FROM user GROUP BY user.intcol ORDER BY COUNT(user.intcol)", + "v3-plan": "unsupported: in scatter query: complex order by expression: count(`user`.intcol)", + "gen4-plan": { + "QueryType": "SELECT", + "Original": "SELECT user.intcol FROM user GROUP BY user.intcol ORDER BY COUNT(user.intcol)", + "Instructions": { + "OperatorType": "Sort", + "Variant": "Memory", + "OrderBy": "1 ASC", + "ResultColumns": 1, + "Inputs": [ + { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "Aggregates": "sum_count(1) AS count(`user`.intcol)", + "GroupBy": "0", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `user`.intcol, count(`user`.intcol) from `user` where 1 != 1 group by `user`.intcol", + "OrderBy": "0 ASC", + "Query": "select `user`.intcol, count(`user`.intcol) from `user` group by `user`.intcol order by `user`.intcol asc", + "Table": "`user`" + } + ] + } + ] + }, + "TablesUsed": [ + "user.user" + ] + } } ] \ No newline at end of file