diff --git a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go index 45f5eeb5164..4ca11372b56 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go +++ b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go @@ -81,20 +81,6 @@ func TestGroupBy(t *testing.T) { mcmp.AssertMatches("select /*vt+ PLANNER=Gen4 */ id6+id7, count(*) k from t3 group by id6+id7 order by k", `[[INT64(9) INT64(1)] [INT64(6) INT64(2)] [INT64(3) INT64(3)]]`) } -func TestDistinct(t *testing.T) { - mcmp, closer := start(t) - defer closer() - mcmp.Exec("insert into t3(id5,id6,id7) values(1,3,3), (2,3,4), (3,3,6), (4,5,7), (5,5,6)") - mcmp.Exec("insert into t7_xxhash(uid,phone) values('1',4), ('2',4), ('3',3), ('4',1), ('5',1)") - mcmp.Exec("insert into aggr_test(id, val1, val2) values(1,'a',1), (2,'A',1), (3,'b',1), (4,'c',3), (5,'c',4)") - mcmp.Exec("insert into aggr_test(id, val1, val2) values(6,'d',null), (7,'e',null), (8,'E',1)") - mcmp.AssertMatches("select distinct val2, count(*) from aggr_test group by val2", `[[NULL INT64(2)] [INT64(1) INT64(4)] [INT64(3) INT64(1)] [INT64(4) INT64(1)]]`) - mcmp.AssertMatches("select distinct id6 from t3 join t7_xxhash on t3.id5 = t7_xxhash.phone", `[[INT64(3)] [INT64(5)]]`) - mcmp.Exec("delete from t3") - mcmp.Exec("delete from t7_xxhash") - mcmp.Exec("delete from aggr_test") -} - func TestEqualFilterOnScatter(t *testing.T) { mcmp, closer := start(t) defer closer() diff --git a/go/test/endtoend/vtgate/queries/aggregation/distinct_test.go b/go/test/endtoend/vtgate/queries/aggregation/distinct_test.go new file mode 100644 index 00000000000..6f17f54979b --- /dev/null +++ b/go/test/endtoend/vtgate/queries/aggregation/distinct_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package aggregation + +import ( + "testing" + + "vitess.io/vitess/go/test/endtoend/utils" +) + +func TestDistinct(t *testing.T) { + mcmp, closer := start(t) + defer closer() + mcmp.Exec("insert into t3(id5,id6,id7) values(1,3,3), (2,3,4), (3,3,6), (4,5,7), (5,5,6)") + mcmp.Exec("insert into t7_xxhash(uid,phone) values('1',4), ('2',4), ('3',3), ('4',1), ('5',1)") + mcmp.Exec("insert into aggr_test(id, val1, val2) values(1,'a',1), (2,'A',1), (3,'b',1), (4,'c',3), (5,'c',4)") + mcmp.Exec("insert into aggr_test(id, val1, val2) values(6,'d',null), (7,'e',null), (8,'E',1)") + mcmp.AssertMatches("select distinct val2, count(*) from aggr_test group by val2", `[[NULL INT64(2)] [INT64(1) INT64(4)] [INT64(3) INT64(1)] [INT64(4) INT64(1)]]`) + mcmp.AssertMatches("select distinct id6 from t3 join t7_xxhash on t3.id5 = t7_xxhash.phone", `[[INT64(3)] [INT64(5)]]`) +} + +func TestDistinctIt(t *testing.T) { + // tests more variations of DISTINCT + mcmp, closer := start(t) + defer closer() + + mcmp.Exec("insert into aggr_test(id, val1, val2) values(1,'a',1), (2,'A',1), (3,'b',1), (4,'c',3), (5,'c',4)") + mcmp.Exec("insert into aggr_test(id, val1, val2) values(6,'d',null), (7,'e',null), (8,'E',1)") + + mcmp.AssertMatchesNoOrder("select distinct val1 from aggr_test", `[[VARCHAR("c")] [VARCHAR("d")] [VARCHAR("e")] [VARCHAR("a")] [VARCHAR("b")]]`) + mcmp.AssertMatchesNoOrder("select distinct val2 from aggr_test", `[[INT64(1)] [INT64(4)] [INT64(3)] [NULL]]`) + mcmp.AssertMatchesNoOrder("select distinct id from aggr_test", `[[INT64(1)] [INT64(2)] [INT64(3)] [INT64(5)] [INT64(4)] [INT64(6)] [INT64(7)] [INT64(8)]]`) + + if utils.BinaryIsAtVersion(17, "vtgate") { + mcmp.AssertMatches("select /*vt+ PLANNER=Gen4 */ distinct val1 from aggr_test order by val1 desc", `[[VARCHAR("e")] [VARCHAR("d")] [VARCHAR("c")] [VARCHAR("b")] [VARCHAR("a")]]`) + mcmp.AssertMatchesNoOrder("select /*vt+ PLANNER=Gen4 */ distinct val1, count(*) from aggr_test group by val1", `[[VARCHAR("a") INT64(2)] [VARCHAR("b") INT64(1)] [VARCHAR("c") INT64(2)] [VARCHAR("d") INT64(1)] [VARCHAR("e") INT64(2)]]`) + mcmp.AssertMatchesNoOrder("select /*vt+ PLANNER=Gen4 */ distinct val1+val2 from aggr_test", `[[NULL] [FLOAT64(1)] [FLOAT64(3)] [FLOAT64(4)]]`) + } +} diff --git a/go/vt/vtgate/planbuilder/operator_transformers.go b/go/vt/vtgate/planbuilder/operator_transformers.go index 9580fd85075..0577fc91c81 100644 --- a/go/vt/vtgate/planbuilder/operator_transformers.go +++ b/go/vt/vtgate/planbuilder/operator_transformers.go @@ -73,6 +73,8 @@ func transformToLogicalPlan(ctx *plancontext.PlanningContext, op ops.Operator, i return transformOrdering(ctx, op) case *operators.Aggregator: return transformAggregator(ctx, op) + case *operators.Distinct: + return transformDistinct(ctx, op) } return nil, vterrors.VT13001(fmt.Sprintf("unknown type encountered: %T (transformToLogicalPlan)", op)) @@ -120,6 +122,14 @@ func transformAggregator(ctx *plancontext.PlanningContext, op *operators.Aggrega return oa, nil } +func transformDistinct(ctx *plancontext.PlanningContext, op *operators.Distinct) (logicalPlan, error) { + src, err := transformToLogicalPlan(ctx, op.Source, false) + if err != nil { + return nil, err + } + return newDistinct(src, op.Columns /*needToTruncate*/, false), nil +} + func transformOrdering(ctx *plancontext.PlanningContext, op *operators.Ordering) (logicalPlan, error) { plan, err := transformToLogicalPlan(ctx, op.Source, false) if err != nil { diff --git a/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go b/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go index 1725d45612f..2783b1dc6f5 100644 --- a/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go +++ b/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go @@ -121,7 +121,7 @@ withNextColumn: continue withNextColumn } } - pushedAggr.addNoPushCol(aeWrap(col), true) + pushedAggr.addColumnWithoutPushing(aeWrap(col), true) } // Set the source of the filter to the new aggregator placed below the route. diff --git a/go/vt/vtgate/planbuilder/operators/aggregator.go b/go/vt/vtgate/planbuilder/operators/aggregator.go index b49a0cb1b07..ac1218ea8fd 100644 --- a/go/vt/vtgate/planbuilder/operators/aggregator.go +++ b/go/vt/vtgate/planbuilder/operators/aggregator.go @@ -91,7 +91,7 @@ func (a *Aggregator) AddPredicate(ctx *plancontext.PlanningContext, expr sqlpars return a, nil } -func (a *Aggregator) addNoPushCol(expr *sqlparser.AliasedExpr, addToGroupBy bool) int { +func (a *Aggregator) addColumnWithoutPushing(expr *sqlparser.AliasedExpr, addToGroupBy bool) int { offset := len(a.Columns) a.Columns = append(a.Columns, expr) diff --git a/go/vt/vtgate/planbuilder/operators/distinct.go b/go/vt/vtgate/planbuilder/operators/distinct.go new file mode 100644 index 00000000000..2dfb105c2e9 --- /dev/null +++ b/go/vt/vtgate/planbuilder/operators/distinct.go @@ -0,0 +1,124 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operators + +import ( + "golang.org/x/exp/slices" + + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/engine" + "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" + "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" +) + +type ( + Distinct struct { + Source ops.Operator + QP *QueryProjection + Pushed bool + + // When offset planning, we'll fill in this field + Columns []engine.CheckCol + } +) + +func (d *Distinct) planOffsets(ctx *plancontext.PlanningContext) error { + columns, err := d.GetColumns() + if err != nil { + return err + } + d.Columns = nil + var exprs []sqlparser.Expr + for _, col := range columns { + newSrc, offset, err := d.Source.AddColumn(ctx, col, true, false) + if err != nil { + return err + } + d.Source = newSrc + e := d.QP.GetSimplifiedExpr(col.Expr) + exprs = append(exprs, e) + d.Columns = append(d.Columns, engine.CheckCol{ + Col: offset, + Collation: ctx.SemTable.CollationForExpr(e), + }) + } + for i, e := range exprs { + if !ctx.SemTable.NeedsWeightString(e) { + continue + } + newSrc, offset, err := d.Source.AddColumn(ctx, aeWrap(weightStringFor(e)), true, false) + if err != nil { + return err + } + d.Source = newSrc + d.Columns[i].WsCol = &offset + } + return nil +} + +func (d *Distinct) Clone(inputs []ops.Operator) ops.Operator { + return &Distinct{ + Source: inputs[0], + Columns: slices.Clone(d.Columns), + QP: d.QP, + Pushed: d.Pushed, + } +} + +func (d *Distinct) Inputs() []ops.Operator { + return []ops.Operator{d.Source} +} + +func (d *Distinct) SetInputs(operators []ops.Operator) { + d.Source = operators[0] +} + +func (d *Distinct) AddPredicate(ctx *plancontext.PlanningContext, expr sqlparser.Expr) (ops.Operator, error) { + newSrc, err := d.Source.AddPredicate(ctx, expr) + if err != nil { + return nil, err + } + d.Source = newSrc + return d, nil +} + +func (d *Distinct) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.AliasedExpr, reuseExisting, addToGroupBy bool) (ops.Operator, int, error) { + newSrc, offset, err := d.Source.AddColumn(ctx, expr, reuseExisting, addToGroupBy) + if err != nil { + return nil, 0, err + } + d.Source = newSrc + return d, offset, nil +} + +func (d *Distinct) GetColumns() ([]*sqlparser.AliasedExpr, error) { + return d.Source.GetColumns() +} + +func (d *Distinct) Description() ops.OpDescription { + return ops.OpDescription{ + OperatorType: "Distinct", + } +} + +func (d *Distinct) ShortDescription() string { + return "" +} + +func (d *Distinct) GetOrdering() ([]ops.OrderBy, error) { + return d.Source.GetOrdering() +} diff --git a/go/vt/vtgate/planbuilder/operators/horizon_planning.go b/go/vt/vtgate/planbuilder/operators/horizon_planning.go index 1d783c9e904..9d6c6bec373 100644 --- a/go/vt/vtgate/planbuilder/operators/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/operators/horizon_planning.go @@ -31,6 +31,21 @@ import ( "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" ) +type ( + projector struct { + cols []ProjExpr + names []*sqlparser.AliasedExpr + } + + // horizonLike should be removed. we should use Horizon for both these cases + horizonLike interface { + ops.Operator + selectStatement() sqlparser.SelectStatement + src() ops.Operator + getQP(ctx *plancontext.PlanningContext) (*QueryProjection, error) + } +) + func errHorizonNotPlanned() error { return _errHorizonNotPlanned } @@ -120,6 +135,8 @@ func optimizeHorizonPlanning(ctx *plancontext.PlanningContext, root ops.Operator return tryPushingDownAggregator(ctx, in) case *Filter: return tryPushingDownFilter(ctx, in) + case *Distinct: + return tryPushingDownDistinct(in) default: return in, rewrite.SameTree, nil } @@ -167,6 +184,37 @@ func tryPushingDownFilter(ctx *plancontext.PlanningContext, in *Filter) (ops.Ope return rewrite.Swap(in, proj, "push filter under projection") } +func tryPushingDownDistinct(in *Distinct) (ops.Operator, *rewrite.ApplyResult, error) { + if in.Pushed { + return in, rewrite.SameTree, nil + } + switch src := in.Source.(type) { + case *Route: + if src.IsSingleShard() { + return rewrite.Swap(in, src, "push distinct under route") + } + case *Distinct: + return src, rewrite.NewTree("removed double distinct", src), nil + } + + cols, err := in.Source.GetColumns() + if err != nil { + return nil, nil, err + } + + aggr := &Aggregator{ + Source: in.Source, + QP: in.QP, + Original: true, + } + + for _, col := range cols { + aggr.addColumnWithoutPushing(col, true) + } + + return aggr, rewrite.NewTree("replace distinct with aggregator", in), nil +} + // addOrderBysAndGroupBysForAggregations runs after we have run horizonPlanning until the op tree stops changing // this means that we have pushed aggregations and other ops as far down as they'll go // addOrderBysAndGroupBysForAggregations will find Aggregators that have not been pushed under routes and @@ -258,7 +306,7 @@ func tryPushingDownOrdering(ctx *plancontext.PlanningContext, in *Ordering) (ops } return rewrite.Swap(in, src, "push ordering under projection") case *Aggregator: - if !src.QP.AlignGroupByAndOrderBy(ctx) { + if !(src.QP.AlignGroupByAndOrderBy(ctx) || overlaps(ctx, in.Order, src.Grouping)) { return in, rewrite.SameTree, nil } @@ -268,6 +316,20 @@ func tryPushingDownOrdering(ctx *plancontext.PlanningContext, in *Ordering) (ops return in, rewrite.SameTree, nil } +func overlaps(ctx *plancontext.PlanningContext, order []ops.OrderBy, grouping []GroupBy) bool { +ordering: + for _, orderBy := range order { + for _, groupBy := range grouping { + if ctx.SemTable.EqualsExprWithDeps(orderBy.SimplifiedExpr, groupBy.SimplifiedExpr) { + continue ordering + } + } + return false + } + + return true +} + func pushOrderingUnderAggr(ctx *plancontext.PlanningContext, order *Ordering, aggregator *Aggregator) (ops.Operator, *rewrite.ApplyResult, error) { // Step 1: Align the GROUP BY and ORDER BY. // Reorder the GROUP BY columns to match the ORDER BY columns. @@ -364,11 +426,6 @@ func pushDownProjectionInVindex( return src, rewrite.NewTree("push projection into vindex", p), nil } -type projector struct { - cols []ProjExpr - names []*sqlparser.AliasedExpr -} - func (p *projector) add(e ProjExpr, alias *sqlparser.AliasedExpr) { p.cols = append(p.cols, e) p.names = append(p.names, alias) @@ -630,25 +687,13 @@ func pushOrExpandHorizon(ctx *plancontext.PlanningContext, in horizonLike) (ops. return expandHorizon(ctx, in) } -// horizonLike should be removed. we should use Horizon for both these cases -type horizonLike interface { - ops.Operator - selectStatement() sqlparser.SelectStatement - src() ops.Operator - getQP(ctx *plancontext.PlanningContext) (*QueryProjection, error) -} - func expandHorizon(ctx *plancontext.PlanningContext, horizon horizonLike) (ops.Operator, *rewrite.ApplyResult, error) { sel, isSel := horizon.selectStatement().(*sqlparser.Select) if !isSel { return nil, nil, errHorizonNotPlanned() } - qp, err := horizon.getQP(ctx) - if err != nil { - return nil, nil, err - } - if sel.Having != nil || qp.NeedsDistinct() || sel.Distinct { + if sel.Having != nil { return nil, nil, errHorizonNotPlanned() } @@ -657,6 +702,25 @@ func expandHorizon(ctx *plancontext.PlanningContext, horizon horizonLike) (ops.O return nil, nil, err } + qp, err := horizon.getQP(ctx) + if err != nil { + return nil, nil, err + } + + if qp.NeedsDistinct() { + op = &Distinct{ + Source: op, + QP: qp, + } + } + + if len(qp.OrderExprs) > 0 { + op = &Ordering{ + Source: op, + Order: qp.OrderExprs, + } + } + if sel.Limit != nil { op = &Limit{ Source: op, @@ -696,12 +760,6 @@ func createProjectionFromSelect(ctx *plancontext.PlanningContext, horizon horizo projX.Alias = derived.Alias } out = projX - if qp.OrderExprs != nil { - out = &Ordering{ - Source: out, - Order: qp.OrderExprs, - } - } return out, nil } @@ -757,14 +815,6 @@ outer: return nil, vterrors.VT13001(fmt.Sprintf("Could not find the %v in aggregation in the original query", expr)) } - // If ordering is required, create an Ordering operation. - if len(qp.OrderExprs) > 0 { - return &Ordering{ - Source: a, - Order: qp.OrderExprs, - }, nil - } - return a, nil } diff --git a/go/vt/vtgate/planbuilder/operators/projection.go b/go/vt/vtgate/planbuilder/operators/projection.go index b8ac95af4ad..d401f696474 100644 --- a/go/vt/vtgate/planbuilder/operators/projection.go +++ b/go/vt/vtgate/planbuilder/operators/projection.go @@ -85,7 +85,7 @@ func (p *Projection) addUnexploredExpr(ae *sqlparser.AliasedExpr, e sqlparser.Ex return len(p.Projections) - 1 } -func (p *Projection) addNoPushCol(expr *sqlparser.AliasedExpr, _ bool) int { +func (p *Projection) addColumnWithoutPushing(expr *sqlparser.AliasedExpr, _ bool) int { return p.addUnexploredExpr(expr, expr.Expr) } diff --git a/go/vt/vtgate/planbuilder/operators/queryprojection.go b/go/vt/vtgate/planbuilder/operators/queryprojection.go index d122330dc32..8d65afe5afa 100644 --- a/go/vt/vtgate/planbuilder/operators/queryprojection.go +++ b/go/vt/vtgate/planbuilder/operators/queryprojection.go @@ -185,11 +185,7 @@ func CreateQPFromSelect(ctx *plancontext.PlanningContext, sel *sqlparser.Select) return nil, err } - if qp.Distinct && !qp.HasAggr { - // grouping and distinct both lead to unique results, so we don't need - // TODO: we should check that we are returning all the grouping columns, or this is not safe to do - qp.groupByExprs = nil - } + qp.calculateDistinct(ctx) return qp, nil } @@ -336,6 +332,38 @@ func (qp *QueryProjection) addOrderBy(ctx *plancontext.PlanningContext, orderBy return nil } +func (qp *QueryProjection) calculateDistinct(ctx *plancontext.PlanningContext) { + if qp.Distinct && !qp.HasAggr { + // grouping and distinct both lead to unique results, so we don't need + qp.groupByExprs = nil + } + + if qp.HasAggr && len(qp.groupByExprs) == 0 { + // this is a scalar aggregation and is inherently distinct + qp.Distinct = false + } + + if !qp.Distinct || len(qp.groupByExprs) == 0 { + return + } + + for _, gb := range qp.groupByExprs { + _, found := canReuseColumn(ctx, qp.SelectExprs, gb.SimplifiedExpr, func(expr SelectExpr) sqlparser.Expr { + getExpr, err := expr.GetExpr() + if err != nil { + panic(err) + } + return getExpr + }) + if !found { + return + } + } + + // since we are returning all grouping expressions, we know the results are guaranteed to be unique + qp.Distinct = false +} + func (qp *QueryProjection) addGroupBy(ctx *plancontext.PlanningContext, groupBy sqlparser.GroupBy) error { es := &expressionSet{} for _, group := range groupBy { diff --git a/go/vt/vtgate/planbuilder/operators/route.go b/go/vt/vtgate/planbuilder/operators/route.go index bcff3ffa652..785371315cc 100644 --- a/go/vt/vtgate/planbuilder/operators/route.go +++ b/go/vt/vtgate/planbuilder/operators/route.go @@ -571,7 +571,7 @@ func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Alia r.Source = src // And since we are under the route, we don't need to continue pushing anything further down - offset := src.addNoPushCol(expr, false) + offset := src.addColumnWithoutPushing(expr, false) if err != nil { return nil, 0, err } @@ -579,7 +579,7 @@ func (r *Route) AddColumn(ctx *plancontext.PlanningContext, expr *sqlparser.Alia } type selectExpressions interface { - addNoPushCol(expr *sqlparser.AliasedExpr, addToGroupBy bool) int + addColumnWithoutPushing(expr *sqlparser.AliasedExpr, addToGroupBy bool) int isDerived() bool } @@ -597,7 +597,7 @@ func addColumnToInput(operator ops.Operator, expr *sqlparser.AliasedExpr, addToG // we have to add a new projection and can't build on this one return false, 0 } - offset := op.addNoPushCol(expr, addToGroupBy) + offset := op.addColumnWithoutPushing(expr, addToGroupBy) return true, offset default: return false, 0 diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index 5d421cc8a3a..fafdbedc856 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -484,9 +484,9 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select col from `user` where 1 != 1", + "FieldQuery": "select col from `user` where 1 != 1 group by col", "OrderBy": "0 ASC", - "Query": "select distinct col from `user` order by col asc", + "Query": "select col from `user` group by col order by col asc", "Table": "`user`" } ] @@ -1797,26 +1797,19 @@ "Original": "select distinct a, count(*) from user", "Instructions": { "OperatorType": "Aggregate", - "Variant": "Ordered", - "GroupBy": "0, 1", + "Variant": "Scalar", + "Aggregates": "random(0) AS a, sum_count_star(1) AS count(*)", "Inputs": [ { - "OperatorType": "Aggregate", - "Variant": "Scalar", - "Aggregates": "random(0) AS a, sum_count_star(1) AS count(*)", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select a, count(*) from `user` where 1 != 1", - "Query": "select a, count(*) from `user`", - "Table": "`user`" - } - ] + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select a, count(*) from `user` where 1 != 1", + "Query": "select a, count(*) from `user`", + "Table": "`user`" } ] }, @@ -1864,28 +1857,21 @@ "Instructions": { "OperatorType": "Aggregate", "Variant": "Ordered", - "GroupBy": "(0|2), 1", + "Aggregates": "sum_count_star(1) AS count(*)", + "GroupBy": "(0|2)", "ResultColumns": 2, "Inputs": [ { - "OperatorType": "Aggregate", - "Variant": "Ordered", - "Aggregates": "sum_count_star(1) AS count(*)", - "GroupBy": "(0|2)", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "Scatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select a, count(*), weight_string(a) from `user` where 1 != 1 group by a, weight_string(a)", - "OrderBy": "(0|2) ASC", - "Query": "select a, count(*), weight_string(a) from `user` group by a, weight_string(a) order by a asc", - "Table": "`user`" - } - ] + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select a, count(*), weight_string(a) from `user` where 1 != 1 group by a, weight_string(a)", + "OrderBy": "(0|2) ASC", + "Query": "select a, count(*), weight_string(a) from `user` group by a, weight_string(a) order by a asc", + "Table": "`user`" } ] }, @@ -2088,9 +2074,9 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select col1, col2, weight_string(col1), weight_string(col2) from `user` where 1 != 1 group by col1, col2", + "FieldQuery": "select col1, col2, weight_string(col1), weight_string(col2) from `user` where 1 != 1 group by col1, col2, weight_string(col1), weight_string(col2)", "OrderBy": "(0|2) ASC, (1|3) ASC", - "Query": "select distinct col1, col2, weight_string(col1), weight_string(col2) from `user` group by col1, col2 order by col1 asc, col2 asc", + "Query": "select col1, col2, weight_string(col1), weight_string(col2) from `user` group by col1, col2, weight_string(col1), weight_string(col2) order by col1 asc, col2 asc", "Table": "`user`" } ] @@ -6174,5 +6160,67 @@ "user.user" ] } + }, { + "comment": "scatter aggregate with ambiguous aliases", + "query": "select distinct a, b as a from user", + "v3-plan": "generating ORDER BY clause: VT03021: ambiguous column reference: a", + "gen4-plan": { + "QueryType": "SELECT", + "Original": "select distinct a, b as a from user", + "Instructions": { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "GroupBy": "(0|2), (1|3)", + "ResultColumns": 2, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select a, b as a, weight_string(a), weight_string(b) from `user` where 1 != 1 group by a, b, weight_string(a), weight_string(b)", + "OrderBy": "(0|2) ASC, (1|3) ASC", + "Query": "select a, b as a, weight_string(a), weight_string(b) from `user` group by a, b, weight_string(a), weight_string(b) order by a asc, b asc", + "Table": "`user`" + } + ] + }, + "TablesUsed": [ + "user.user" + ] + } +}, { + "comment": "scatter aggregate with complex select list (can't build order by)", + "query": "select distinct a+1 from user", + "v3-plan": "generating ORDER BY clause: VT12001: unsupported: reference a complex expression", + "gen4-plan": { + "QueryType": "SELECT", + "Original": "select distinct a+1 from user", + "Instructions": { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "GroupBy": "(0|1)", + "ResultColumns": 1, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select a + 1, weight_string(a + 1) from `user` where 1 != 1 group by a + 1, weight_string(a + 1)", + "OrderBy": "(0|1) ASC", + "Query": "select a + 1, weight_string(a + 1) from `user` group by a + 1, weight_string(a + 1) order by a + 1 asc", + "Table": "`user`" + } + ] + }, + "TablesUsed": [ + "user.user" + ] } +} ] diff --git a/go/vt/vtgate/planbuilder/testdata/oltp_cases.json b/go/vt/vtgate/planbuilder/testdata/oltp_cases.json index 88717292379..0ee69603f25 100644 --- a/go/vt/vtgate/planbuilder/testdata/oltp_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/oltp_cases.json @@ -206,8 +206,7 @@ "Instructions": { "OperatorType": "Aggregate", "Variant": "Ordered", - "GroupBy": "(0|1) COLLATE latin1_swedish_ci", - "ResultColumns": 1, + "GroupBy": "0 COLLATE latin1_swedish_ci", "Inputs": [ { "OperatorType": "Route", @@ -216,9 +215,9 @@ "Name": "main", "Sharded": true }, - "FieldQuery": "select c, weight_string(c) from sbtest30 where 1 != 1", - "OrderBy": "0 ASC COLLATE latin1_swedish_ci, 0 ASC COLLATE latin1_swedish_ci", - "Query": "select distinct c, weight_string(c) from sbtest30 where id between 1 and 10 order by c asc, c asc", + "FieldQuery": "select c from sbtest30 where 1 != 1 group by c", + "OrderBy": "0 ASC COLLATE latin1_swedish_ci", + "Query": "select c from sbtest30 where id between 1 and 10 group by c order by c asc", "Table": "sbtest30" } ] @@ -404,4 +403,4 @@ ] } } -] \ No newline at end of file +] diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json index 0efd31d29b8..d17035fe880 100644 --- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.json @@ -2957,9 +2957,9 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select `user`.a, weight_string(`user`.a) from `user` where 1 != 1", + "FieldQuery": "select `user`.a, weight_string(`user`.a) from `user` where 1 != 1 group by `user`.a, weight_string(`user`.a)", "OrderBy": "(0|1) ASC", - "Query": "select `user`.a, weight_string(`user`.a) from `user` order by `user`.a asc", + "Query": "select `user`.a, weight_string(`user`.a) from `user` group by `user`.a, weight_string(`user`.a) order by `user`.a asc", "Table": "`user`" }, { @@ -2969,8 +2969,8 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select 1 from user_extra where 1 != 1", - "Query": "select 1 from user_extra", + "FieldQuery": "select 1 from user_extra where 1 != 1 group by .0", + "Query": "select 1 from user_extra group by .0", "Table": "user_extra" } ] @@ -3026,9 +3026,9 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select a as c, a, weight_string(a) from `user` where 1 != 1", + "FieldQuery": "select a as c, a, weight_string(a) from `user` where 1 != 1 group by a, a, weight_string(a)", "OrderBy": "(0|2) ASC, (0|2) ASC", - "Query": "select distinct a as c, a, weight_string(a) from `user` order by c asc, a asc", + "Query": "select a as c, a, weight_string(a) from `user` group by a, a, weight_string(a) order by a asc, a asc", "Table": "`user`" } ] @@ -3058,9 +3058,9 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select a, a, weight_string(a) from `user` where 1 != 1", + "FieldQuery": "select a, a, weight_string(a) from `user` where 1 != 1 group by a, a, weight_string(a)", "OrderBy": "(0|2) ASC, (0|2) ASC", - "Query": "select distinct a, a, weight_string(a) from `user` order by a asc, a asc", + "Query": "select a, a, weight_string(a) from `user` group by a, a, weight_string(a) order by a asc, a asc", "Table": "`user`" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json index d88bcb93ff3..9f5a875ed12 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json @@ -375,12 +375,6 @@ "v3-plan": "VT12001: unsupported: in scatter query: complex aggregate expression", "gen4-plan": "VT12001: unsupported: in scatter query: aggregation function 'avg(id)'" }, - { - "comment": "scatter aggregate with ambiguous aliases", - "query": "select distinct a, b as a from user", - "v3-plan": "generating ORDER BY clause: VT03021: ambiguous column reference: a", - "gen4-plan": "VT13001: [BUG] generating ORDER BY clause: ambiguous symbol reference: a" - }, { "comment": "outer and inner subquery route reference the same \"uu.id\" name\n# but they refer to different things. The first reference is to the outermost query,\n# and the second reference is to the innermost 'from' subquery.\n# This query will never work as the inner derived table is only selecting one of the column", "query": "select id2 from user uu where id in (select id from user where id = uu.id and user.col in (select col from (select id from user_extra where user_id = 5) uu where uu.user_id = uu.id))", @@ -417,12 +411,6 @@ "query": "with x as (select * from user) select * from x union select * from x", "plan": "VT12001: unsupported: WITH expression in UNION statement" }, - { - "comment": "scatter aggregate with complex select list (can't build order by)", - "query": "select distinct a+1 from user", - "v3-plan": "generating ORDER BY clause: VT12001: unsupported: reference a complex expression", - "gen4-plan": "VT13001: [BUG] in scatter query: complex ORDER BY expression: a + 1" - }, { "comment": "aggregation on union", "query": "select sum(col) from (select col from user union all select col from unsharded) t",