diff --git a/go/mysql/sql_error.go b/go/mysql/sql_error.go index fc436cd7788..509663ae78f 100644 --- a/go/mysql/sql_error.go +++ b/go/mysql/sql_error.go @@ -191,6 +191,7 @@ var stateToMysqlCode = map[vterrors.State]struct { vterrors.WrongNumberOfColumnsInSelect: {num: ERWrongNumberOfColumnsInSelect, state: SSWrongNumberOfColumns}, vterrors.WrongTypeForVar: {num: ERWrongTypeForVar, state: SSClientError}, vterrors.WrongValueForVar: {num: ERWrongValueForVar, state: SSClientError}, + vterrors.WrongFieldWithGroup: {num: ERWrongFieldWithGroup, state: SSClientError}, vterrors.ServerNotAvailable: {num: ERServerIsntAvailable, state: SSNetError}, vterrors.CantDoThisInTransaction: {num: ERCantDoThisDuringAnTransaction, state: SSCantDoThisDuringAnTransaction}, vterrors.RequiresPrimaryKey: {num: ERRequiresPrimaryKey, state: SSClientError}, diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 624ce3a39e4..de6df303125 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -1368,16 +1368,10 @@ func ToString(exprs []TableExpr) string { } // ContainsAggregation returns true if the expression contains aggregation -func ContainsAggregation(e Expr) bool { +func ContainsAggregation(e SQLNode) bool { hasAggregates := false _ = Walk(func(node SQLNode) (kontinue bool, err error) { - switch node := node.(type) { - case *FuncExpr: - if node.IsAggregate() { - hasAggregates = true - return false, nil - } - case *GroupConcatExpr: + if IsAggregation(node) { hasAggregates = true return false, nil } @@ -1385,3 +1379,14 @@ func ContainsAggregation(e Expr) bool { }, e) return hasAggregates } + +// IsAggregation returns true if the node is an aggregation expression +func IsAggregation(node SQLNode) bool { + switch node := node.(type) { + case *FuncExpr: + return node.IsAggregate() + case *GroupConcatExpr: + return true + } + return false +} diff --git a/go/vt/vterrors/state.go b/go/vt/vterrors/state.go index 40ec8390db0..00c73231d51 100644 --- a/go/vt/vterrors/state.go +++ b/go/vt/vterrors/state.go @@ -35,6 +35,7 @@ const ( NonUniqTable NonUpdateableTable SyntaxError + WrongFieldWithGroup WrongGroupField WrongTypeForVar WrongValueForVar diff --git a/go/vt/vtgate/planbuilder/abstract/queryprojection.go b/go/vt/vtgate/planbuilder/abstract/queryprojection.go index 2e97aa9a735..d8a0be7b875 100644 --- a/go/vt/vtgate/planbuilder/abstract/queryprojection.go +++ b/go/vt/vtgate/planbuilder/abstract/queryprojection.go @@ -38,6 +38,7 @@ type ( // If you change the contents here, please update the toString() method SelectExprs []SelectExpr HasAggr bool + Distinct bool GroupByExprs []GroupBy OrderExprs []OrderBy CanPushDownSorting bool @@ -58,39 +59,28 @@ type ( // CreateQPFromSelect created the QueryProjection for the input *sqlparser.Select func CreateQPFromSelect(sel *sqlparser.Select) (*QueryProjection, error) { - qp := &QueryProjection{} + qp := &QueryProjection{ + Distinct: sel.Distinct, + } - hasNonAggr := false for _, selExp := range sel.SelectExprs { exp, ok := selExp.(*sqlparser.AliasedExpr) if !ok { return nil, semantics.Gen4NotSupportedF("%T in select list", selExp) } - fExpr, ok := exp.Expr.(*sqlparser.FuncExpr) - if ok && fExpr.IsAggregate() { - if len(fExpr.Exprs) != 1 { - return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "aggregate functions take a single argument '%s'", sqlparser.String(fExpr)) - } - if fExpr.Distinct { - return nil, semantics.Gen4NotSupportedF("distinct aggregation") - } - qp.HasAggr = true - qp.SelectExprs = append(qp.SelectExprs, SelectExpr{ - Col: exp, - Aggr: true, - }) - continue - } + if err := checkForInvalidAggregations(exp); err != nil { + return nil, err + } + col := SelectExpr{ + Col: exp, + } if sqlparser.ContainsAggregation(exp.Expr) { - return nil, vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: in scatter query: complex aggregate expression") + col.Aggr = true + qp.HasAggr = true } - hasNonAggr = true - qp.SelectExprs = append(qp.SelectExprs, SelectExpr{Col: exp}) - } - if hasNonAggr && qp.HasAggr && sel.GroupBy == nil { - return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.MixOfGroupFuncAndFields, "Mixing of aggregation and non-aggregation columns is not allowed if there is no GROUP BY clause") + qp.SelectExprs = append(qp.SelectExprs, col) } for _, group := range sel.GroupBy { @@ -104,6 +94,18 @@ func CreateQPFromSelect(sel *sqlparser.Select) (*QueryProjection, error) { qp.GroupByExprs = append(qp.GroupByExprs, GroupBy{Inner: expr, WeightStrExpr: weightStrExpr}) } + if qp.HasAggr { + expr := qp.getNonAggrExprNotMatchingGroupByExprs() + // if we have aggregation functions, non aggregating columns and GROUP BY, + // the non-aggregating expressions must all be listed in the GROUP BY list + if expr != nil { + if len(qp.GroupByExprs) > 0 { + return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.WrongFieldWithGroup, "Expression of SELECT list is not in GROUP BY clause and contains nonaggregated column '%s' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by", sqlparser.String(expr)) + } + return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.MixOfGroupFuncAndFields, "In aggregated query without GROUP BY, expression of SELECT list contains nonaggregated column '%s'; this is incompatible with sql_mode=only_full_group_by", sqlparser.String(expr)) + } + } + canPushDownSorting := true for _, order := range sel.OrderBy { expr, weightStrExpr, err := qp.getSimplifiedExpr(order.Expr, "order clause") @@ -119,11 +121,47 @@ func CreateQPFromSelect(sel *sqlparser.Select) (*QueryProjection, error) { }) canPushDownSorting = canPushDownSorting && !sqlparser.ContainsAggregation(weightStrExpr) } + if qp.Distinct && !qp.HasAggr { + qp.GroupByExprs = nil + } qp.CanPushDownSorting = canPushDownSorting - return qp, nil } +func checkForInvalidAggregations(exp *sqlparser.AliasedExpr) error { + return sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + fExpr, ok := node.(*sqlparser.FuncExpr) + if ok && fExpr.IsAggregate() { + if len(fExpr.Exprs) != 1 { + return false, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "aggregate functions take a single argument '%s'", sqlparser.String(fExpr)) + } + if fExpr.Distinct { + return false, semantics.Gen4NotSupportedF("distinct aggregation") + } + } + return true, nil + }, exp.Expr) +} + +func (qp *QueryProjection) getNonAggrExprNotMatchingGroupByExprs() sqlparser.Expr { + for _, expr := range qp.SelectExprs { + if expr.Aggr { + continue + } + isGroupByOk := false + for _, groupByExpr := range qp.GroupByExprs { + if sqlparser.EqualsExpr(groupByExpr.WeightStrExpr, expr.Col.Expr) { + isGroupByOk = true + break + } + } + if !isGroupByOk { + return expr.Col.Expr + } + } + return nil +} + // getSimplifiedExpr takes an expression used in ORDER BY or GROUP BY, which can reference both aliased columns and // column offsets, and returns an expression that is simpler to evaluate func (qp *QueryProjection) getSimplifiedExpr(e sqlparser.Expr, caller string) (expr sqlparser.Expr, weightStrExpr sqlparser.Expr, err error) { @@ -169,11 +207,13 @@ func (qp *QueryProjection) toString() string { Select []string Grouping []string OrderBy []string + Distinct bool } out := output{ Select: []string{}, Grouping: []string{}, OrderBy: []string{}, + Distinct: qp.NeedsDistinct(), } for _, expr := range qp.SelectExprs { @@ -204,3 +244,26 @@ func (qp *QueryProjection) toString() string { func (qp *QueryProjection) NeedsAggregation() bool { return qp.HasAggr || len(qp.GroupByExprs) > 0 } + +func (qp QueryProjection) onlyAggr() bool { + if !qp.HasAggr { + return false + } + for _, expr := range qp.SelectExprs { + if !expr.Aggr { + return false + } + } + return true +} + +// NeedsDistinct returns true if the query needs explicit distinct +func (qp *QueryProjection) NeedsDistinct() bool { + if !qp.Distinct { + return false + } + if qp.onlyAggr() && len(qp.GroupByExprs) == 0 { + return false + } + return true +} diff --git a/go/vt/vtgate/planbuilder/abstract/queryprojection_test.go b/go/vt/vtgate/planbuilder/abstract/queryprojection_test.go index e1d90c148ae..57e5ab03ce2 100644 --- a/go/vt/vtgate/planbuilder/abstract/queryprojection_test.go +++ b/go/vt/vtgate/planbuilder/abstract/queryprojection_test.go @@ -46,7 +46,7 @@ func TestQP(t *testing.T) { }, { sql: "select 1, count(1) from user", - expErr: "Mixing of aggregation and non-aggregation columns is not allowed if there is no GROUP BY clause", + expErr: "In aggregated query without GROUP BY, expression of SELECT list contains nonaggregated column '1'; this is incompatible with sql_mode=only_full_group_by", }, { sql: "select max(id) from user", @@ -55,13 +55,9 @@ func TestQP(t *testing.T) { sql: "select max(a, b) from user", expErr: "aggregate functions take a single argument 'max(a, b)'", }, - { - sql: "select func(max(id)) from user", - expErr: "unsupported: in scatter query: complex aggregate expression", - }, { sql: "select 1, count(1) from user order by 1", - expErr: "Mixing of aggregation and non-aggregation columns is not allowed if there is no GROUP BY clause", + expErr: "In aggregated query without GROUP BY, expression of SELECT list contains nonaggregated column '1'; this is incompatible with sql_mode=only_full_group_by", }, { sql: "select id from user order by col, id, 1", @@ -132,7 +128,8 @@ func TestQPSimplifiedExpr(t *testing.T) { "Grouping": [ "intcol" ], - "OrderBy": [] + "OrderBy": [], + "Distinct": false }`, }, { @@ -147,7 +144,8 @@ func TestQPSimplifiedExpr(t *testing.T) { "OrderBy": [ "intcol asc", "textcol asc" - ] + ], + "Distinct": false }`, }, { @@ -166,7 +164,33 @@ func TestQPSimplifiedExpr(t *testing.T) { ], "OrderBy": [ "textcol desc" - ] + ], + "Distinct": false +}`, + }, + { + query: "select distinct col1, col2 from user group by col1, col2", + expected: ` +{ + "Select": [ + "col1", + "col2" + ], + "Grouping": [], + "OrderBy": [], + "Distinct": true +}`, + }, + { + query: "select distinct count(*) from user", + expected: ` +{ + "Select": [ + "aggr: count(*)" + ], + "Grouping": [], + "OrderBy": [], + "Distinct": false }`, }, } diff --git a/go/vt/vtgate/planbuilder/distinct.go b/go/vt/vtgate/planbuilder/distinct.go index b3642eaef09..666d39ac2ee 100644 --- a/go/vt/vtgate/planbuilder/distinct.go +++ b/go/vt/vtgate/planbuilder/distinct.go @@ -24,11 +24,7 @@ import ( var _ logicalPlan = (*distinct)(nil) -// limit is the logicalPlan for engine.Limit. -// This gets built if a limit needs to be applied -// after rows are returned from an underlying -// operation. Since a limit is the final operation -// of a SELECT, most pushes are not applicable. +// distinct is the logicalPlan for engine.Distinct. type distinct struct { logicalPlanCommon } diff --git a/go/vt/vtgate/planbuilder/selectGen4.go b/go/vt/vtgate/planbuilder/horizon_planning.go similarity index 77% rename from go/vt/vtgate/planbuilder/selectGen4.go rename to go/vt/vtgate/planbuilder/horizon_planning.go index 583620301a0..dc27578b58b 100644 --- a/go/vt/vtgate/planbuilder/selectGen4.go +++ b/go/vt/vtgate/planbuilder/horizon_planning.go @@ -121,29 +121,37 @@ func checkIfAlreadyExists(expr *sqlparser.AliasedExpr, sel *sqlparser.Select) in return -1 } -func planAggregations(qp *abstract.QueryProjection, plan logicalPlan, semTable *semantics.SemTable, vschema ContextVSchema) (logicalPlan, bool, bool, error) { - newPlan := plan +func (hp *horizonPlanning) haveToTruncate(v bool) { + hp.needsTruncation = hp.needsTruncation || v +} + +func (hp *horizonPlanning) planAggregations() error { + newPlan := hp.plan var oa *orderedAggregate - if !hasUniqueVindex(vschema, semTable, qp.GroupByExprs) { + if !hasUniqueVindex(hp.vschema, hp.semTable, hp.qp.GroupByExprs) { eaggr := &engine.OrderedAggregate{} oa = &orderedAggregate{ resultsBuilder: resultsBuilder{ - logicalPlanCommon: newBuilderCommon(plan), + logicalPlanCommon: newBuilderCommon(hp.plan), weightStrings: make(map[*resultColumn]int), truncater: eaggr, }, eaggr: eaggr, } newPlan = oa + hp.vtgateGrouping = true } - for _, e := range qp.SelectExprs { - offset, _, err := pushProjection(e.Col, plan, semTable, true, false) + for _, e := range hp.qp.SelectExprs { + offset, _, err := pushProjection(e.Col, hp.plan, hp.semTable, true, false) if err != nil { - return nil, false, false, err + return err } if e.Aggr && oa != nil { - fExpr := e.Col.Expr.(*sqlparser.FuncExpr) + fExpr, isFunc := e.Col.Expr.(*sqlparser.FuncExpr) + if !isFunc { + return vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: in scatter query: complex aggregate expression") + } opcode := engine.SupportedAggregates[fExpr.Name.Lowered()] oa.eaggr.Aggregates = append(oa.eaggr.Aggregates, engine.AggregateParams{ Opcode: opcode, @@ -153,62 +161,53 @@ func planAggregations(qp *abstract.QueryProjection, plan logicalPlan, semTable * } } - var colAdded bool - for _, groupExpr := range qp.GroupByExprs { - added, err := planGroupByGen4(groupExpr, newPlan, semTable) + for _, groupExpr := range hp.qp.GroupByExprs { + added, err := planGroupByGen4(groupExpr, newPlan, hp.semTable) if err != nil { - return nil, false, false, err + return err } - colAdded = colAdded || added + hp.haveToTruncate(added) } - if !qp.CanPushDownSorting && oa != nil { + if !hp.qp.CanPushDownSorting && oa != nil { var orderExprs []abstract.OrderBy // if we can't at a later stage push down the sorting to our inputs, we have to do ordering here - for _, groupExpr := range qp.GroupByExprs { + for _, groupExpr := range hp.qp.GroupByExprs { orderExprs = append(orderExprs, abstract.OrderBy{ Inner: &sqlparser.Order{Expr: groupExpr.Inner}, WeightStrExpr: groupExpr.WeightStrExpr}, ) } if len(orderExprs) > 0 { - newInput, added, err := planOrderBy(qp, orderExprs, plan, semTable) + newInput, err := hp.planOrderBy(orderExprs, hp.plan) if err != nil { - return nil, false, false, err + return err } oa.input = newInput - return oa, colAdded || added, true, nil + hp.plan = oa + } + } else { + hp.plan = newPlan + } + + // done with aggregation planning. let's check if we should fail the query + if _, planIsRoute := hp.plan.(*route); !planIsRoute { + // if we had to build up additional operators around the route, we have to fail this query + for _, expr := range hp.qp.SelectExprs { + colExpr := expr.Col.Expr + if !sqlparser.IsAggregation(colExpr) && sqlparser.ContainsAggregation(colExpr) { + return vterrors.New(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: in scatter query: complex aggregate expression") + } } } - return newPlan, colAdded, oa != nil, nil + + return nil } func hasUniqueVindex(vschema ContextVSchema, semTable *semantics.SemTable, groupByExprs []abstract.GroupBy) bool { for _, groupByExpr := range groupByExprs { - col, isCol := groupByExpr.WeightStrExpr.(*sqlparser.ColName) - if !isCol { - continue - } - ts := semTable.Dependencies(groupByExpr.WeightStrExpr) - tableInfo, err := semTable.TableInfoFor(ts) - if err != nil { - continue - } - tableName, err := tableInfo.Name() - if err != nil { - continue - } - vschemaTable, _, _, _, _, err := vschema.FindTableOrVindex(tableName) - if err != nil { - continue - } - for _, vindex := range vschemaTable.ColumnVindexes { - if len(vindex.Columns) > 1 || !vindex.Vindex.IsUnique() { - continue - } - if col.Name.Equal(vindex.Columns[0]) { - return true - } + if exprHasUniqueVindex(vschema, semTable, groupByExpr.WeightStrExpr) { + return true } } return false @@ -236,11 +235,11 @@ func planGroupByGen4(groupExpr abstract.GroupBy, plan logicalPlan, semTable *sem } } -func planOrderByUsingGroupBy(qp *abstract.QueryProjection, plan logicalPlan, semTable *semantics.SemTable) (logicalPlan, bool, error) { +func (hp *horizonPlanning) planOrderByUsingGroupBy() (logicalPlan, error) { var orderExprs []abstract.OrderBy - for _, groupExpr := range qp.GroupByExprs { + for _, groupExpr := range hp.qp.GroupByExprs { addExpr := true - for _, orderExpr := range qp.OrderExprs { + for _, orderExpr := range hp.qp.OrderExprs { if sqlparser.EqualsExpr(groupExpr.Inner, orderExpr.Inner.Expr) { addExpr = false break @@ -254,37 +253,47 @@ func planOrderByUsingGroupBy(qp *abstract.QueryProjection, plan logicalPlan, sem } } if len(orderExprs) > 0 { - return planOrderBy(qp, orderExprs, plan, semTable) + return hp.planOrderBy(orderExprs, hp.plan) } - return plan, false, nil + return hp.plan, nil } -func planOrderBy(qp *abstract.QueryProjection, orderExprs []abstract.OrderBy, plan logicalPlan, semTable *semantics.SemTable) (logicalPlan, bool, error) { +func (hp *horizonPlanning) planOrderBy(orderExprs []abstract.OrderBy, plan logicalPlan) (logicalPlan, error) { switch plan := plan.(type) { case *route: - return planOrderByForRoute(orderExprs, plan, semTable) + newPlan, truncate, err := planOrderByForRoute(orderExprs, plan, hp.semTable) + if err != nil { + return nil, err + } + hp.haveToTruncate(truncate) + return newPlan, nil case *joinGen4: - return planOrderByForJoin(qp, orderExprs, plan, semTable) + newPlan, err := hp.planOrderByForJoin(orderExprs, plan) + if err != nil { + return nil, err + } + + return newPlan, nil case *orderedAggregate: for _, order := range orderExprs { if sqlparser.ContainsAggregation(order.WeightStrExpr) { ms, err := createMemorySortPlanOnAggregation(plan, orderExprs) if err != nil { - return nil, false, err + return nil, err } - return ms, false, nil + return ms, nil } } - newInput, colAdded, err := planOrderBy(qp, orderExprs, plan.input, semTable) + newInput, err := hp.planOrderBy(orderExprs, plan.input) if err != nil { - return nil, false, err + return nil, err } plan.input = newInput - return plan, colAdded, nil + return plan, nil case *memorySort: - return plan, false, nil + return plan, nil default: - return nil, false, semantics.Gen4NotSupportedF("ordering on complex query %T", plan) + return nil, semantics.Gen4NotSupportedF("ordering on complex query %T", plan) } } @@ -354,16 +363,22 @@ func needsWeightString(tbl semantics.TableInfo, colName *sqlparser.ColName) bool return true // we didn't find the column. better to add just to be safe1 } -func planOrderByForJoin(qp *abstract.QueryProjection, orderExprs []abstract.OrderBy, plan *joinGen4, semTable *semantics.SemTable) (logicalPlan, bool, error) { - if allLeft(orderExprs, semTable, plan.Left.ContainsTables()) { - newLeft, _, err := planOrderBy(qp, orderExprs, plan.Left, semTable) +func (hp *horizonPlanning) planOrderByForJoin(orderExprs []abstract.OrderBy, plan *joinGen4) (logicalPlan, error) { + if allLeft(orderExprs, hp.semTable, plan.Left.ContainsTables()) { + newLeft, err := hp.planOrderBy(orderExprs, plan.Left) if err != nil { - return nil, false, err + return nil, err } plan.Left = newLeft - return plan, false, nil + hp.needsTruncation = false // since this is a join, we can safely + // add extra columns and not need to truncate them + return plan, nil } - return createMemorySortPlan(plan, orderExprs, semTable) + sortPlan, err := hp.createMemorySortPlan(plan, orderExprs) + if err != nil { + return nil, err + } + return sortPlan, nil } func createMemorySortPlanOnAggregation(plan *orderedAggregate, orderExprs []abstract.OrderBy) (logicalPlan, error) { @@ -406,7 +421,7 @@ func findExprInOrderedAggr(plan *orderedAggregate, order abstract.OrderBy) (int, return 0, 0, false } -func createMemorySortPlan(plan logicalPlan, orderExprs []abstract.OrderBy, semTable *semantics.SemTable) (logicalPlan, bool, error) { +func (hp *horizonPlanning) createMemorySortPlan(plan logicalPlan, orderExprs []abstract.OrderBy) (logicalPlan, error) { primitive := &engine.MemorySort{} ms := &memorySort{ resultsBuilder: resultsBuilder{ @@ -417,13 +432,12 @@ func createMemorySortPlan(plan logicalPlan, orderExprs []abstract.OrderBy, semTa eMemorySort: primitive, } - var colAdded bool for _, order := range orderExprs { - offset, weightStringOffset, added, err := wrapAndPushExpr(order.Inner.Expr, order.WeightStrExpr, plan, semTable) + offset, weightStringOffset, added, err := wrapAndPushExpr(order.Inner.Expr, order.WeightStrExpr, plan, hp.semTable) if err != nil { - return nil, false, err + return nil, err } - colAdded = colAdded || added + hp.haveToTruncate(added) ms.eMemorySort.OrderBy = append(ms.eMemorySort.OrderBy, engine.OrderByParams{ Col: offset, WeightStringCol: weightStringOffset, @@ -431,7 +445,7 @@ func createMemorySortPlan(plan logicalPlan, orderExprs []abstract.OrderBy, semTa StarColFixedIndex: offset, }) } - return ms, colAdded, nil + return ms, nil } func allLeft(orderExprs []abstract.OrderBy, semTable *semantics.SemTable, lhsTables semantics.TableSet) bool { diff --git a/go/vt/vtgate/planbuilder/jointree.go b/go/vt/vtgate/planbuilder/jointree.go index 91ff3c66751..8aedd07e00e 100644 --- a/go/vt/vtgate/planbuilder/jointree.go +++ b/go/vt/vtgate/planbuilder/jointree.go @@ -46,7 +46,7 @@ type ( tableNames() []string } - leJoin struct { + joinTables struct { lhs, rhs relation pred sqlparser.Expr } @@ -128,14 +128,14 @@ type ( var _ joinTree = (*routePlan)(nil) var _ joinTree = (*joinPlan)(nil) var _ relation = (*routeTable)(nil) -var _ relation = (*leJoin)(nil) +var _ relation = (*joinTables)(nil) var _ relation = (parenTables)(nil) func (rp *routeTable) tableID() semantics.TableSet { return rp.qtable.TableID } -func (rp *leJoin) tableID() semantics.TableSet { return rp.lhs.tableID().Merge(rp.rhs.tableID()) } +func (rp *joinTables) tableID() semantics.TableSet { return rp.lhs.tableID().Merge(rp.rhs.tableID()) } -func (rp *leJoin) tableNames() []string { +func (rp *joinTables) tableNames() []string { return append(rp.lhs.tableNames(), rp.rhs.tableNames()...) } @@ -181,7 +181,7 @@ func visitTables(r relation, f func(tbl *routeTable) error) error { } } return nil - case *leJoin: + case *joinTables: err := visitTables(r.lhs, f) if err != nil { return err diff --git a/go/vt/vtgate/planbuilder/jointree_transformers.go b/go/vt/vtgate/planbuilder/jointree_transformers.go index a99c0fb16be..dfa98d649b8 100644 --- a/go/vt/vtgate/planbuilder/jointree_transformers.go +++ b/go/vt/vtgate/planbuilder/jointree_transformers.go @@ -186,7 +186,7 @@ func relToTableExpr(t relation) (sqlparser.TableExpr, error) { tables = append(tables, tableExpr) } return &sqlparser.ParenTableExpr{Exprs: tables}, nil - case *leJoin: + case *joinTables: lExpr, err := relToTableExpr(t.lhs) if err != nil { return nil, err diff --git a/go/vt/vtgate/planbuilder/ordered_aggregate.go b/go/vt/vtgate/planbuilder/ordered_aggregate.go index d7e9a0a29bc..cf4bbed2e42 100644 --- a/go/vt/vtgate/planbuilder/ordered_aggregate.go +++ b/go/vt/vtgate/planbuilder/ordered_aggregate.go @@ -74,8 +74,7 @@ func (pb *primitiveBuilder) checkAggregates(sel *sqlparser.Select) error { } // Check if we can allow aggregates. - hasAggregates := nodeHasAggregates(sel.SelectExprs) || len(sel.GroupBy) > 0 - + hasAggregates := sqlparser.ContainsAggregation(sel.SelectExprs) || len(sel.GroupBy) > 0 if !hasAggregates && !sel.Distinct { return nil } @@ -134,27 +133,6 @@ func (pb *primitiveBuilder) checkAggregates(sel *sqlparser.Select) error { return nil } -func nodeHasAggregates(node sqlparser.SQLNode) bool { - hasAggregates := false - _ = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { - switch node := node.(type) { - case *sqlparser.FuncExpr: - if node.IsAggregate() { - hasAggregates = true - return false, errors.New("unused error") - } - case *sqlparser.GroupConcatExpr: - hasAggregates = true - return false, errors.New("unused error") - case *sqlparser.Subquery: - // Subqueries are analyzed by themselves. - return false, nil - } - return true, nil - }, node) - return hasAggregates -} - // groupbyHasUniqueVindex looks ahead at the group by expression to see if // it references a unique vindex. // diff --git a/go/vt/vtgate/planbuilder/project.go b/go/vt/vtgate/planbuilder/project.go index 3a1989cfee4..669118346e4 100644 --- a/go/vt/vtgate/planbuilder/project.go +++ b/go/vt/vtgate/planbuilder/project.go @@ -83,7 +83,7 @@ func planProjection(pb *primitiveBuilder, in logicalPlan, expr *sqlparser.Aliase } // Ensure that there are no aggregates in the expression. - if nodeHasAggregates(expr.Expr) { + if sqlparser.ContainsAggregation(expr.Expr) { return nil, nil, 0, errors.New("unsupported: in scatter query: complex aggregate expression") } diff --git a/go/vt/vtgate/planbuilder/route_planning.go b/go/vt/vtgate/planbuilder/route_planning.go index 59f9dade24f..91903c2af39 100644 --- a/go/vt/vtgate/planbuilder/route_planning.go +++ b/go/vt/vtgate/planbuilder/route_planning.go @@ -80,7 +80,14 @@ func newBuildSelectPlan(sel *sqlparser.Select, vschema ContextVSchema) (engine.P return nil, err } - plan, err = planHorizon(sel, plan, semTable, vschema) + hp := horizonPlanning{ + sel: sel, + plan: plan, + semTable: semTable, + vschema: vschema, + } + + plan, err = hp.planHorizon() if err != nil { return nil, err } @@ -154,72 +161,153 @@ func planLimit(limit *sqlparser.Limit, plan logicalPlan) (logicalPlan, error) { return lPlan, nil } -func planHorizon(sel *sqlparser.Select, plan logicalPlan, semTable *semantics.SemTable, vschema ContextVSchema) (logicalPlan, error) { - rb, ok := plan.(*route) - if !ok && semTable.ProjectionErr != nil { - return nil, semTable.ProjectionErr +type horizonPlanning struct { + sel *sqlparser.Select + plan logicalPlan + semTable *semantics.SemTable + vschema ContextVSchema + qp *abstract.QueryProjection + needsTruncation bool + vtgateGrouping bool +} + +func (hp horizonPlanning) planHorizon() (logicalPlan, error) { + rb, ok := hp.plan.(*route) + if !ok && hp.semTable.ProjectionErr != nil { + return nil, hp.semTable.ProjectionErr } if ok && rb.isSingleShard() { - createSingleShardRoutePlan(sel, rb) - return plan, nil + createSingleShardRoutePlan(hp.sel, rb) + return hp.plan, nil } - if err := checkUnsupportedConstructs(sel); err != nil { + qp2, err := abstract.CreateQPFromSelect(hp.sel) + if err != nil { return nil, err } - qp, err := abstract.CreateQPFromSelect(sel) - if err != nil { + hp.qp = qp2 + + if err := checkUnsupportedConstructs(hp.sel); err != nil { return nil, err } - var needsTruncation, vtgateGrouping bool - if qp.NeedsAggregation() { - plan, needsTruncation, vtgateGrouping, err = planAggregations(qp, plan, semTable, vschema) + if hp.qp.NeedsAggregation() { + err = hp.planAggregations() if err != nil { return nil, err } } else { - for _, e := range qp.SelectExprs { - if _, _, err := pushProjection(e.Col, plan, semTable, true, false); err != nil { + for _, e := range hp.qp.SelectExprs { + if _, _, err := pushProjection(e.Col, hp.plan, hp.semTable, true, false); err != nil { return nil, err } } } - if len(qp.OrderExprs) > 0 { - var colAdded bool - plan, colAdded, err = planOrderBy(qp, qp.OrderExprs, plan, semTable) + if len(hp.qp.OrderExprs) > 0 { + hp.plan, err = hp.planOrderBy(hp.qp.OrderExprs, hp.plan) if err != nil { return nil, err } - needsTruncation = needsTruncation || colAdded } - if qp.CanPushDownSorting && vtgateGrouping { - var colAdded bool - plan, colAdded, err = planOrderByUsingGroupBy(qp, plan, semTable) + if hp.qp.CanPushDownSorting && hp.vtgateGrouping { + hp.plan, err = hp.planOrderByUsingGroupBy() if err != nil { return nil, err } - needsTruncation = needsTruncation || colAdded } - if needsTruncation { - switch p := plan.(type) { - case *route: - p.eroute.SetTruncateColumnCount(sel.GetColumnCount()) - case *orderedAggregate: - p.eaggr.SetTruncateColumnCount(sel.GetColumnCount()) - case *memorySort: - p.truncater.SetTruncateColumnCount(sel.GetColumnCount()) - default: - return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "plan type not known for column truncation: %T", plan) + err = hp.truncateColumnsIfNeeded() + if err != nil { + return nil, err + } + + if hp.qp.NeedsDistinct() { + hp.plan, err = pushDistinct(hp.plan, hp.semTable, hp.vschema, hp.qp) + if err != nil { + return nil, err } } - return plan, nil + return hp.plan, nil +} + +func (hp horizonPlanning) truncateColumnsIfNeeded() error { + if !hp.needsTruncation { + return nil + } + + switch p := hp.plan.(type) { + case *route: + p.eroute.SetTruncateColumnCount(hp.sel.GetColumnCount()) + case *orderedAggregate: + p.eaggr.SetTruncateColumnCount(hp.sel.GetColumnCount()) + case *memorySort: + p.truncater.SetTruncateColumnCount(hp.sel.GetColumnCount()) + default: + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "plan type not known for column truncation: %T", hp.plan) + } + + return nil +} + +func pushDistinct(plan logicalPlan, semTable *semantics.SemTable, vschema ContextVSchema, qp *abstract.QueryProjection) (logicalPlan, error) { + switch p := plan.(type) { + case *route: + // we always make the underlying query distinct, + // and then we might also add a distinct operator on top if it is needed + p.Select.MakeDistinct() + if !p.isSingleShard() && !selectHasUniqueVindex(vschema, semTable, qp.SelectExprs) { + plan = newDistinct(plan) + } + return plan, nil + case *orderedAggregate, *joinGen4: + return newDistinct(plan), nil + + default: + return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "unknown plan type for DISTINCT %T", plan) + } +} + +func selectHasUniqueVindex(vschema ContextVSchema, semTable *semantics.SemTable, sel []abstract.SelectExpr) bool { + for _, expr := range sel { + if exprHasUniqueVindex(vschema, semTable, expr.Col.Expr) { + return true + } + } + return false +} + +func exprHasUniqueVindex(vschema ContextVSchema, semTable *semantics.SemTable, expr sqlparser.Expr) bool { + col, isCol := expr.(*sqlparser.ColName) + if !isCol { + return false + } + ts := semTable.Dependencies(expr) + tableInfo, err := semTable.TableInfoFor(ts) + if err != nil { + return false + } + tableName, err := tableInfo.Name() + if err != nil { + return false + } + vschemaTable, _, _, _, _, err := vschema.FindTableOrVindex(tableName) + if err != nil { + return false + } + for _, vindex := range vschemaTable.ColumnVindexes { + if len(vindex.Columns) > 1 || !vindex.Vindex.IsUnique() { + return false + } + if col.Name.Equal(vindex.Columns[0]) { + return true + } + } + return false } func createSingleShardRoutePlan(sel *sqlparser.Select, rb *route) { @@ -237,9 +325,6 @@ func createSingleShardRoutePlan(sel *sqlparser.Select, rb *route) { } func checkUnsupportedConstructs(sel *sqlparser.Select) error { - if sel.Distinct { - return semantics.Gen4NotSupportedF("DISTINCT") - } if sel.Having != nil { return semantics.Gen4NotSupportedF("HAVING") } @@ -721,7 +806,7 @@ func createRoutePlanForOuter(aRoute, bRoute *routePlan, semTable *semantics.SemT aTbl, bTbl, newTables := findTables(deps, tables) tables = newTables if aTbl != nil && bTbl != nil { - tables = append(tables, &leJoin{ + tables = append(tables, &joinTables{ lhs: aTbl, rhs: bTbl, pred: predicate, diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt index c8caa2a717a..373cde13a9b 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.txt @@ -78,8 +78,9 @@ Gen4 plan same as above "Table": "`user`" } } +Gen4 plan same as above -# distinct and group by together for single route. +# distinct and group by together for single route - group by is redundant "select distinct col1, id from user group by col1" { "QueryType": "SELECT", @@ -96,6 +97,21 @@ Gen4 plan same as above "Table": "`user`" } } +{ + "QueryType": "SELECT", + "Original": "select distinct col1, id from user group by col1", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col1, id from `user` where 1 != 1", + "Query": "select distinct col1, id from `user`", + "Table": "`user`" + } +} # scatter group by a text column "select count(*), a, textcol1, b from user group by a, textcol1, b" @@ -378,6 +394,26 @@ Gen4 plan same as above ] } } +{ + "QueryType": "SELECT", + "Original": "select distinct col1, col2 from user group by col1", + "Instructions": { + "OperatorType": "Distinct", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col1, col2 from `user` where 1 != 1", + "Query": "select distinct col1, col2 from `user`", + "Table": "`user`" + } + ] + } +} # aggregate on RHS subquery (tests symbol table merge) "select user.a, t.b from user join (select count(*) b from unsharded) as t" @@ -606,6 +642,7 @@ Gen4 plan same as above "Table": "`user`" } } +Gen4 plan same as above # group by a unique vindex where alias from select list is used "select id as val, 1+count(*) from user group by val" @@ -624,6 +661,7 @@ Gen4 plan same as above "Table": "`user`" } } +Gen4 plan same as above # group by a unique vindex where expression is qualified (alias should be ignored) "select val as id, 1+count(*) from user group by user.id" @@ -642,6 +680,7 @@ Gen4 plan same as above "Table": "`user`" } } +Gen4 error: Expression of SELECT list is not in GROUP BY clause and contains nonaggregated column 'val' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by # group by a unique vindex where it should skip non-aliased expressions. "select *, id, 1+count(*) from user group by id" @@ -740,6 +779,7 @@ Gen4 plan same as above ] } } +Gen4 error: In aggregated query without GROUP BY, expression of SELECT list contains nonaggregated column 'id'; this is incompatible with sql_mode=only_full_group_by # scatter aggregate using distinct "select distinct col from user" @@ -767,6 +807,26 @@ Gen4 plan same as above ] } } +{ + "QueryType": "SELECT", + "Original": "select distinct col from user", + "Instructions": { + "OperatorType": "Distinct", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col from `user` where 1 != 1", + "Query": "select distinct col from `user`", + "Table": "`user`" + } + ] + } +} # scatter aggregate group by select col "select col from user group by col" @@ -1177,16 +1237,15 @@ Gen4 plan same as above ] } } - -# scatter aggregate group by column number -"select col from user group by 1" { "QueryType": "SELECT", - "Original": "select col from user group by 1", + "Original": "select a, b, count(*) from user group by b, a", "Instructions": { "OperatorType": "Aggregate", "Variant": "Ordered", - "GroupBy": "0", + "Aggregates": "count(2)", + "GroupBy": "(1|3), (0|4)", + "ResultColumns": 3, "Inputs": [ { "OperatorType": "Route", @@ -1195,30 +1254,24 @@ Gen4 plan same as above "Name": "user", "Sharded": true }, - "FieldQuery": "select col, weight_string(col) from `user` where 1 != 1 group by 1", - "OrderBy": "(0|1) ASC", - "Query": "select col, weight_string(col) from `user` group by 1 order by col asc", - "ResultColumns": 1, + "FieldQuery": "select a, b, count(*), weight_string(b), weight_string(a) from `user` where 1 != 1 group by b, a", + "OrderBy": "(1|3) ASC, (0|4) ASC", + "Query": "select a, b, count(*), weight_string(b), weight_string(a) from `user` group by b, a order by b asc, a asc", "Table": "`user`" } ] } } -# scatter aggregate group by invalid column number -"select col from user group by 2" -"Unknown column '2' in 'group statement'" -Gen4 plan same as above - -# scatter aggregate order by null -"select count(*) from user order by null" +# scatter aggregate group by column number +"select col from user group by 1" { "QueryType": "SELECT", - "Original": "select count(*) from user order by null", + "Original": "select col from user group by 1", "Instructions": { "OperatorType": "Aggregate", "Variant": "Ordered", - "Aggregates": "count(0)", + "GroupBy": "0", "Inputs": [ { "OperatorType": "Route", @@ -1227,28 +1280,23 @@ Gen4 plan same as above "Name": "user", "Sharded": true }, - "FieldQuery": "select count(*) from `user` where 1 != 1", - "Query": "select count(*) from `user`", + "FieldQuery": "select col, weight_string(col) from `user` where 1 != 1 group by 1", + "OrderBy": "(0|1) ASC", + "Query": "select col, weight_string(col) from `user` group by 1 order by col asc", + "ResultColumns": 1, "Table": "`user`" } ] } } - -# scatter aggregate with complex select list (can't build order by) -"select distinct a+1 from user" -"generating order by clause: cannot reference a complex expression" - -# scatter aggregate with numbered order by columns -"select a, b, c, d, count(*) from user group by 1, 2, 3 order by 1, 2, 3" { "QueryType": "SELECT", - "Original": "select a, b, c, d, count(*) from user group by 1, 2, 3 order by 1, 2, 3", + "Original": "select col from user group by 1", "Instructions": { "OperatorType": "Aggregate", "Variant": "Ordered", - "Aggregates": "count(4)", - "GroupBy": "0, 1, 2", + "GroupBy": "(0|1)", + "ResultColumns": 1, "Inputs": [ { "OperatorType": "Route", @@ -1257,24 +1305,29 @@ Gen4 plan same as above "Name": "user", "Sharded": true }, - "FieldQuery": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` where 1 != 1 group by 1, 2, 3", - "OrderBy": "(0|5) ASC, (1|6) ASC, (2|7) ASC", - "Query": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` group by 1, 2, 3 order by 1 asc, 2 asc, 3 asc", - "ResultColumns": 5, + "FieldQuery": "select col, weight_string(col) from `user` where 1 != 1 group by col", + "OrderBy": "(0|1) ASC", + "Query": "select col, weight_string(col) from `user` group by col order by col asc", "Table": "`user`" } ] } } + +# scatter aggregate group by invalid column number +"select col from user group by 2" +"Unknown column '2' in 'group statement'" +Gen4 plan same as above + +# scatter aggregate order by null +"select count(*) from user order by null" { "QueryType": "SELECT", - "Original": "select a, b, c, d, count(*) from user group by 1, 2, 3 order by 1, 2, 3", + "Original": "select count(*) from user order by null", "Instructions": { "OperatorType": "Aggregate", "Variant": "Ordered", - "Aggregates": "count(4)", - "GroupBy": "(0|5), (1|6), (2|7)", - "ResultColumns": 5, + "Aggregates": "count(0)", "Inputs": [ { "OperatorType": "Route", @@ -1283,20 +1336,24 @@ Gen4 plan same as above "Name": "user", "Sharded": true }, - "FieldQuery": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` where 1 != 1 group by a, b, c", - "OrderBy": "(0|5) ASC, (1|6) ASC, (2|7) ASC", - "Query": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` group by a, b, c order by a asc, b asc, c asc", + "FieldQuery": "select count(*) from `user` where 1 != 1", + "Query": "select count(*) from `user`", "Table": "`user`" } ] } } -# scatter aggregate with named order by columns -"select a, b, c, d, count(*) from user group by 1, 2, 3 order by a, b, c" +# scatter aggregate with complex select list (can't build order by) +"select distinct a+1 from user" +"generating order by clause: cannot reference a complex expression" + + +# scatter aggregate with numbered order by columns +"select a, b, c, d, count(*) from user group by 1, 2, 3 order by 1, 2, 3" { "QueryType": "SELECT", - "Original": "select a, b, c, d, count(*) from user group by 1, 2, 3 order by a, b, c", + "Original": "select a, b, c, d, count(*) from user group by 1, 2, 3 order by 1, 2, 3", "Instructions": { "OperatorType": "Aggregate", "Variant": "Ordered", @@ -1312,13 +1369,17 @@ Gen4 plan same as above }, "FieldQuery": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` where 1 != 1 group by 1, 2, 3", "OrderBy": "(0|5) ASC, (1|6) ASC, (2|7) ASC", - "Query": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` group by 1, 2, 3 order by a asc, b asc, c asc", + "Query": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` group by 1, 2, 3 order by 1 asc, 2 asc, 3 asc", "ResultColumns": 5, "Table": "`user`" } ] } } +Gen4 error: Expression of SELECT list is not in GROUP BY clause and contains nonaggregated column 'd' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by + +# scatter aggregate with named order by columns +"select a, b, c, d, count(*) from user group by 1, 2, 3 order by a, b, c" { "QueryType": "SELECT", "Original": "select a, b, c, d, count(*) from user group by 1, 2, 3 order by a, b, c", @@ -1326,8 +1387,7 @@ Gen4 plan same as above "OperatorType": "Aggregate", "Variant": "Ordered", "Aggregates": "count(4)", - "GroupBy": "(0|5), (1|6), (2|7)", - "ResultColumns": 5, + "GroupBy": "0, 1, 2", "Inputs": [ { "OperatorType": "Route", @@ -1336,14 +1396,16 @@ Gen4 plan same as above "Name": "user", "Sharded": true }, - "FieldQuery": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` where 1 != 1 group by a, b, c", + "FieldQuery": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` where 1 != 1 group by 1, 2, 3", "OrderBy": "(0|5) ASC, (1|6) ASC, (2|7) ASC", - "Query": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` group by a, b, c order by a asc, b asc, c asc", + "Query": "select a, b, c, d, count(*), weight_string(a), weight_string(b), weight_string(c) from `user` group by 1, 2, 3 order by a asc, b asc, c asc", + "ResultColumns": 5, "Table": "`user`" } ] } } +Gen4 error: Expression of SELECT list is not in GROUP BY clause and contains nonaggregated column 'd' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by # scatter aggregate with jumbled order by columns "select a, b, c, d, count(*) from user group by 1, 2, 3, 4 order by d, b, a, c" @@ -1542,6 +1604,37 @@ Gen4 plan same as above ] } } +{ + "QueryType": "SELECT", + "Original": "select col, count(*) from user group by col limit 10", + "Instructions": { + "OperatorType": "Limit", + "Count": 10, + "Inputs": [ + { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "Aggregates": "count(1)", + "GroupBy": "(0|2)", + "ResultColumns": 2, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col, count(*), weight_string(col) from `user` where 1 != 1 group by col", + "OrderBy": "(0|2) ASC", + "Query": "select col, count(*), weight_string(col) from `user` group by col order by col asc limit :__upper_limit", + "Table": "`user`" + } + ] + } + ] + } +} # Group by with collate operator "select user.col1 as a from user where user.id = 5 group by a collate utf8_general_ci" @@ -1636,6 +1729,7 @@ Gen4 plan same as above ] } } +Gen4 error: In aggregated query without GROUP BY, expression of SELECT list contains nonaggregated column 'a'; this is incompatible with sql_mode=only_full_group_by # distinct and aggregate functions "select distinct a, count(*) from user group by a" @@ -1669,6 +1763,36 @@ Gen4 plan same as above ] } } +{ + "QueryType": "SELECT", + "Original": "select distinct a, count(*) from user group by a", + "Instructions": { + "OperatorType": "Distinct", + "Inputs": [ + { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "Aggregates": "count(1)", + "GroupBy": "(0|2)", + "ResultColumns": 2, + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select a, count(*), weight_string(a) from `user` where 1 != 1 group by a", + "OrderBy": "(0|2) ASC", + "Query": "select a, count(*), weight_string(a) from `user` group by a order by a asc", + "Table": "`user`" + } + ] + } + ] + } +} # Group by invalid column number (code is duplicated from symab). "select id from user group by 1.1" @@ -1683,3 +1807,102 @@ Gen4 plan same as above "select count(distinct *) from user" "syntax error: count(distinct *)" Gen4 plan same as above + +# optimize group by when using distinct with no aggregation +"select distinct col1, col2 from user group by col1, col2" +{ + "QueryType": "SELECT", + "Original": "select distinct col1, col2 from user group by col1, col2", + "Instructions": { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "GroupBy": "0, 1, 0, 1", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col1, col2, weight_string(col1), weight_string(col2) from `user` where 1 != 1 group by col1, col2", + "OrderBy": "(0|2) ASC, (1|3) ASC, (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, col1 asc, col2 asc", + "ResultColumns": 2, + "Table": "`user`" + } + ] + } +} +{ + "QueryType": "SELECT", + "Original": "select distinct col1, col2 from user group by col1, col2", + "Instructions": { + "OperatorType": "Distinct", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select col1, col2 from `user` where 1 != 1", + "Query": "select distinct col1, col2 from `user`", + "Table": "`user`" + } + ] + } +} + +# do not use distinct when using only aggregates and no group by +"select distinct count(*) from user" +{ + "QueryType": "SELECT", + "Original": "select distinct count(*) from user", + "Instructions": { + "OperatorType": "Distinct", + "Inputs": [ + { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "Aggregates": "count(0)", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select count(*) from `user` where 1 != 1", + "Query": "select count(*) from `user`", + "Table": "`user`" + } + ] + } + ] + } +} +{ + "QueryType": "SELECT", + "Original": "select distinct count(*) from user", + "Instructions": { + "OperatorType": "Aggregate", + "Variant": "Ordered", + "Aggregates": "count(0)", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select count(*) from `user` where 1 != 1", + "Query": "select count(*) from `user`", + "Table": "`user`" + } + ] + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.txt b/go/vt/vtgate/planbuilder/testdata/onecase.txt index 16a368ddafd..e819513f354 100644 --- a/go/vt/vtgate/planbuilder/testdata/onecase.txt +++ b/go/vt/vtgate/planbuilder/testdata/onecase.txt @@ -1 +1 @@ -# Add your test case here for debugging and run go test -run=One. \ No newline at end of file +# Add your test case here for debugging and run go test -run=One. diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt index a06bec71dc1..3d956547db4 100644 --- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt @@ -1546,3 +1546,63 @@ Gen4 plan same as above ] } } + +# join order by with ambiguous column reference ; valid in MySQL +"select name, name from user, music order by name" +"ambiguous symbol reference: `name`" +{ + "QueryType": "SELECT", + "Original": "select name, name from user, music order by name", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "-1,-1", + "TableName": "`user`_music", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select `name`, weight_string(`name`) from `user` where 1 != 1", + "OrderBy": "(0|1) ASC", + "Query": "select `name`, weight_string(`name`) from `user` order by `name` asc", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select 1 from music where 1 != 1", + "Query": "select 1 from music", + "Table": "music" + } + ] + } +} + +# order by with ambiguous column reference ; valid in MySQL +"select id, id from user order by id" +"ambiguous symbol reference: id" +{ + "QueryType": "SELECT", + "Original": "select id, id from user order by id", + "Instructions": { + "OperatorType": "Route", + "Variant": "SelectScatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select id, id, weight_string(id) from `user` where 1 != 1", + "OrderBy": "(0|2) ASC", + "Query": "select id, id, weight_string(id) from `user` order by id asc", + "ResultColumns": 2, + "Table": "`user`" + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt index 9da1cad5785..c4992c4b7ac 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt @@ -1687,10 +1687,10 @@ Gen4 plan same as above } # select from unsharded keyspace into outfile -"select * from main.unsharded into outfile 'x.txt' character set binary fields terminated by 'term' optionally enclosed by 'c' escaped by 'e' lines starting by 'a' terminated by '\n'" +"select * from main.unsharded into outfile 'x.txt' character set binary fields terminated by 'term' optionally enclosed by 'c' escaped by 'e' lines starting by 'a' terminated by '\\n'" { "QueryType": "SELECT", - "Original": "select * from main.unsharded into outfile 'x.txt' character set binary fields terminated by 'term' optionally enclosed by 'c' escaped by 'e' lines starting by 'a' terminated by '\n'", + "Original": "select * from main.unsharded into outfile 'x.txt' character set binary fields terminated by 'term' optionally enclosed by 'c' escaped by 'e' lines starting by 'a' terminated by '\\n'", "Instructions": { "OperatorType": "Route", "Variant": "SelectUnsharded", @@ -1772,6 +1772,7 @@ Gen4 plan same as above ] } } +Gen4 plan same as above # Select from information schema query with two tables that route should be merged "SELECT DELETE_RULE, UPDATE_RULE FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS KCU INNER JOIN INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS AS RC ON KCU.CONSTRAINT_NAME = RC.CONSTRAINT_NAME WHERE KCU.TABLE_SCHEMA = 'test' AND KCU.TABLE_NAME = 'data_type_table' AND KCU.COLUMN_NAME = 'id' AND KCU.REFERENCED_TABLE_SCHEMA = 'test' AND KCU.CONSTRAINT_NAME = 'data_type_table_id_fkey' ORDER BY KCU.CONSTRAINT_NAME, KCU.COLUMN_NAME" diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt index f0b32471c79..f8d35069956 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt @@ -118,66 +118,6 @@ Gen4 error: aggregate functions take a single argument 'count(a, b)' "select id, b as id, count(*) from user order by id" "ambiguous symbol reference: id" -# order by with ambiguous column reference ; valid in MySQL -"select id, id from user order by id" -"ambiguous symbol reference: id" -{ - "QueryType": "SELECT", - "Original": "select id, id from user order by id", - "Instructions": { - "OperatorType": "Route", - "Variant": "SelectScatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select id, id, weight_string(id) from `user` where 1 != 1", - "OrderBy": "(0|2) ASC", - "Query": "select id, id, weight_string(id) from `user` order by id asc", - "ResultColumns": 2, - "Table": "`user`" - } -} - -# join order by with ambiguous column reference ; valid in MySQL -"select name, name from user, music order by name" -"ambiguous symbol reference: `name`" -{ - "QueryType": "SELECT", - "Original": "select name, name from user, music order by name", - "Instructions": { - "OperatorType": "Join", - "Variant": "Join", - "JoinColumnIndexes": "-1,-1", - "TableName": "`user`_music", - "Inputs": [ - { - "OperatorType": "Route", - "Variant": "SelectScatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select `name`, weight_string(`name`) from `user` where 1 != 1", - "OrderBy": "(0|1) ASC", - "Query": "select `name`, weight_string(`name`) from `user` order by `name` asc", - "Table": "`user`" - }, - { - "OperatorType": "Route", - "Variant": "SelectScatter", - "Keyspace": { - "Name": "user", - "Sharded": true - }, - "FieldQuery": "select 1 from music where 1 != 1", - "Query": "select 1 from music", - "Table": "music" - } - ] - } -} - # scatter aggregate with ambiguous aliases "select distinct a, b as a from user" "generating order by clause: ambiguous symbol reference: a"