From 695b3b5be9ecae29167c9d5366e28ea07d3c3073 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 6 Nov 2023 08:18:36 +0100 Subject: [PATCH] make column resolution closer to mysql Signed-off-by: Andres Taylor --- .../planbuilder/abstract/queryprojection.go | 60 ++++++++------ go/vt/vtgate/planbuilder/horizon_planning.go | 20 ++--- .../planbuilder/testdata/aggr_cases.json | 8 +- .../planbuilder/testdata/select_cases.json | 79 +++++++++++++++++++ 4 files changed, 122 insertions(+), 45 deletions(-) diff --git a/go/vt/vtgate/planbuilder/abstract/queryprojection.go b/go/vt/vtgate/planbuilder/abstract/queryprojection.go index 9b7b9df34ce..8ae35d55e49 100644 --- a/go/vt/vtgate/planbuilder/abstract/queryprojection.go +++ b/go/vt/vtgate/planbuilder/abstract/queryprojection.go @@ -151,17 +151,15 @@ func CreateQPFromSelect(sel *sqlparser.Select) (*QueryProjection, error) { } for _, group := range sel.GroupBy { selectExprIdx, aliasExpr := qp.FindSelectExprIndexForExpr(group) - expr, weightStrExpr, err := qp.GetSimplifiedExpr(group) - if err != nil { - return nil, err - } + selectExprIdx, aliasExpr := qp.FindSelectExprIndexForExpr(ctx, group) + weightStrExpr := qp.GetSimplifiedExpr(group) err = checkForInvalidGroupingExpressions(weightStrExpr) if err != nil { return nil, err } groupBy := GroupBy{ - Inner: expr, + Inner: group, WeightStrExpr: weightStrExpr, InnerIndex: selectExprIdx, aliasedExpr: aliasExpr, @@ -274,17 +272,14 @@ func CreateQPFromUnion(union *sqlparser.Union) (*QueryProjection, error) { func (qp *QueryProjection) addOrderBy(orderBy sqlparser.OrderBy) error { canPushDownSorting := true for _, order := range orderBy { - expr, weightStrExpr, err := qp.GetSimplifiedExpr(order.Expr) - if err != nil { - return err - } + weightStrExpr := qp.GetSimplifiedExpr(order.Expr) if sqlparser.IsNull(weightStrExpr) { // ORDER BY null can safely be ignored continue } qp.OrderExprs = append(qp.OrderExprs, OrderBy{ Inner: &sqlparser.Order{ - Expr: expr, + Expr: order.Expr, Direction: order.Direction, }, WeightStrExpr: weightStrExpr, @@ -360,34 +355,47 @@ func (qp *QueryProjection) isExprInGroupByExprs(expr SelectExpr) bool { } // GetSimplifiedExpr takes an expression used in ORDER BY or GROUP BY, and returns an expression that is simpler to evaluate -func (qp *QueryProjection) GetSimplifiedExpr(e sqlparser.Expr) (expr sqlparser.Expr, weightStrExpr sqlparser.Expr, err error) { +func (qp *QueryProjection) GetSimplifiedExpr(e sqlparser.Expr) (found sqlparser.Expr) { + if qp == nil { + return e + } // If the ORDER BY is against a column alias, we need to remember the expression // behind the alias. The weightstring(.) calls needs to be done against that expression and not the alias. // Eg - select music.foo as bar, weightstring(music.foo) from music order by bar - colExpr, isColName := e.(*sqlparser.ColName) - if !isColName { - return e, e, nil - } - - if sqlparser.IsNull(e) { - return e, nil, nil + in, isColName := e.(*sqlparser.ColName) + if !(isColName && in.Qualifier.IsEmpty()) { + // we are only interested in unqualified column names. if it's not a column name and not unqualified, we're done + return e } - if colExpr.Qualifier.IsEmpty() { - for _, selectExpr := range qp.SelectExprs { - aliasedExpr, isAliasedExpr := selectExpr.Col.(*sqlparser.AliasedExpr) - if !isAliasedExpr { + for _, selectExpr := range qp.SelectExprs { + ae, ok := selectExpr.Col.(*sqlparser.AliasedExpr) + if !ok { + continue + } + aliased := !ae.As.IsEmpty() + if aliased { + if in.Name.Equal(ae.As) { + return ae.Expr + } + } else { + seCol, ok := ae.Expr.(*sqlparser.ColName) + if !ok { continue } - isAliasExpr := !aliasedExpr.As.IsEmpty() - if isAliasExpr && colExpr.Name.Equal(aliasedExpr.As) { - return e, aliasedExpr.Expr, nil + if seCol.Name.Equal(in.Name) { + // If the column name matches, we have a match, even if the table name is not listed + return ae.Expr } } } - return e, e, nil + if found == nil { + found = e + } + + return found } // toString should only be used for tests diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go index 0f3b25a70b4..d9f94bb802e 100644 --- a/go/vt/vtgate/planbuilder/horizon_planning.go +++ b/go/vt/vtgate/planbuilder/horizon_planning.go @@ -535,16 +535,9 @@ func (hp *horizonPlanning) handleDistinctAggr(ctx *plancontext.PlanningContext, aggrs = append(aggrs, expr) continue } - funcExpr := expr.Original.Expr.(*sqlparser.FuncExpr) // we wouldn't be in this method if this wasn't a function - aliasedExpr, ok := funcExpr.Exprs[0].(*sqlparser.AliasedExpr) - if !ok { - err = vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "syntax error: %s", sqlparser.String(expr.Original)) - return - } - inner, innerWS, err := hp.qp.GetSimplifiedExpr(aliasedExpr.Expr) - if err != nil { - return nil, nil, nil, err - } + + inner := expr.Func.GetArg() + innerWS := hp.qp.GetSimplifiedExpr(inner) if exprHasVindex(ctx.SemTable, innerWS, false) { aggrs = append(aggrs, expr) continue @@ -600,13 +593,10 @@ func newOffset(col int) offsets { func (hp *horizonPlanning) createGroupingsForColumns(columns []*sqlparser.ColName) ([]abstract.GroupBy, error) { var lhsGrouping []abstract.GroupBy for _, lhsColumn := range columns { - expr, wsExpr, err := hp.qp.GetSimplifiedExpr(lhsColumn) - if err != nil { - return nil, err - } + wsExpr := hp.qp.GetSimplifiedExpr(lhsColumn) lhsGrouping = append(lhsGrouping, abstract.GroupBy{ - Inner: expr, + Inner: lhsColumn, WeightStrExpr: wsExpr, }) } diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index 6016622d2e3..df0ba86e27e 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -4784,9 +4784,9 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select id, b as id, count(*), weight_string(b) from `user` where 1 != 1", + "FieldQuery": "select id, b as id, count(*), weight_string(id) from `user` where 1 != 1", "OrderBy": "(1|3) ASC", - "Query": "select id, b as id, count(*), weight_string(b) from `user` order by id asc", + "Query": "select id, b as id, count(*), weight_string(id) from `user` order by id asc", "Table": "`user`" } ] @@ -4876,9 +4876,9 @@ "Name": "user", "Sharded": true }, - "FieldQuery": "select `user`.id, weight_string(id) from `user` where 1 != 1 group by id, weight_string(id)", + "FieldQuery": "select id, weight_string(`user`.id) from `user` where 1 != 1 group by id, weight_string(`user`.id)", "OrderBy": "(0|1) ASC", - "Query": "select `user`.id, weight_string(id) from `user` group by id, weight_string(id) order by id asc", + "Query": "select id, weight_string(`user`.id) from `user` group by id, weight_string(`user`.id) order by id asc", "Table": "`user`" }, { diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json index d6b7a8c57bd..17ced9c57a0 100644 --- a/go/vt/vtgate/planbuilder/testdata/select_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json @@ -5684,5 +5684,84 @@ "Table": "dual, unsharded_a" } } + }, + { + "comment": "column with qualifier is correctly used", + "query": "select u.foo, ue.foo as apa from user u, user_extra ue order by foo ", + "v3-plan": { + "QueryType": "SELECT", + "Original": "select u.foo, ue.foo as apa from user u, user_extra ue order by foo ", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "L:0,R:0", + "TableName": "`user`_user_extra", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u.foo, weight_string(u.foo) from `user` as u where 1 != 1", + "OrderBy": "(0|1) ASC", + "Query": "select u.foo, weight_string(u.foo) from `user` as u order by foo asc", + "ResultColumns": 1, + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select ue.foo as apa from user_extra as ue where 1 != 1", + "Query": "select ue.foo as apa from user_extra as ue", + "Table": "user_extra" + } + ] + } + }, + "gen4-plan": { + "QueryType": "SELECT", + "Original": "select u.foo, ue.foo as apa from user u, user_extra ue order by foo ", + "Instructions": { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "L:0,R:0", + "TableName": "`user`_user_extra", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select u.foo, weight_string(u.foo) from `user` as u where 1 != 1", + "OrderBy": "(0|1) ASC", + "Query": "select u.foo, weight_string(u.foo) from `user` as u order by foo asc", + "Table": "`user`" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select ue.foo as apa from user_extra as ue where 1 != 1", + "Query": "select ue.foo as apa from user_extra as ue", + "Table": "user_extra" + } + ] + }, + "TablesUsed": [ + "user.user", + "user.user_extra" + ] + } } ]