slack-vitess-r14.0.5: backport vitessio#14428 from v16 (#165)

* make column resolution closer to mysql Signed-off-by: Andres Taylor <[email protected]> * Fix bad merge conflict fix Signed-off-by: Tim Vaillancourt <[email protected]> * Possible horizon_planning.go fix Signed-off-by: Tim Vaillancourt <[email protected]> * Fix aggr_cases.json plan Signed-off-by: Tim Vaillancourt <[email protected]> * Fix group by order Signed-off-by: Tim Vaillancourt <[email protected]> * Remove 'TablesUsed', not in v14 Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Andres Taylor <[email protected]> Signed-off-by: Tim Vaillancourt <[email protected]> Co-authored-by: Andres Taylor <[email protected]>
slackhq · Nov 12, 2023 · 0d1c5cc · 0d1c5cc
1 parent a9f2532
commit 0d1c5cc
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 45 deletions.
diff --git a/go/vt/vtgate/planbuilder/abstract/queryprojection.go b/go/vt/vtgate/planbuilder/abstract/queryprojection.go
@@ -151,17 +151,14 @@ 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
-		}
+		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 +271,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 +354,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
@@ -541,10 +541,8 @@ func (hp *horizonPlanning) handleDistinctAggr(ctx *plancontext.PlanningContext,
 			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 := aliasedExpr.Expr
+		innerWS := hp.qp.GetSimplifiedExpr(inner)
 		if exprHasVindex(ctx.SemTable, innerWS, false) {
 			aggrs = append(aggrs, expr)
 			continue
@@ -600,13 +598,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
@@ -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`"
           }
         ]
@@ -4851,22 +4851,20 @@
       "Instructions": {
         "OperatorType": "Aggregate",
         "Variant": "Ordered",
-        "Aggregates": "random(0) AS id",
-        "GroupBy": "(2|1)",
+        "GroupBy": "(1|0)",
         "ResultColumns": 1,
         "Inputs": [
           {
             "OperatorType": "Projection",
             "Expressions": [
-              "[COLUMN 2] as id",
               "[COLUMN 1]",
               "[COLUMN 0] as id"
             ],
             "Inputs": [
               {
                 "OperatorType": "Join",
                 "Variant": "Join",
-                "JoinColumnIndexes": "L:0,L:1,L:0",
+                "JoinColumnIndexes": "L:0,L:1",
                 "TableName": "`user`_user_extra",
                 "Inputs": [
                   {
@@ -4876,9 +4874,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`"
                   },
                   {
@@ -4888,8 +4886,8 @@
                       "Name": "user",
                       "Sharded": true
                     },
-                    "FieldQuery": "select 1 from user_extra where 1 != 1 group by 1",
-                    "Query": "select 1 from user_extra group by 1",
+                    "FieldQuery": "select 1 from user_extra where 1 != 1",
+                    "Query": "select 1 from user_extra",
                     "Table": "user_extra"
                   }
                 ]

diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json
@@ -5684,5 +5684,80 @@
         "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"
+          }
+        ]
+      }
+    }
   }
 ]