Repository: vitessio/vitess
Pull request #10079 (2022-04-12 15:51:16)

[13.0] Fix Gen4 only_full_group_by regression
## Description

This Pull Request backports the changes made in https://github.com/vitessio/vitess/pull/10069.

## Checklist
- [x] Tests were added or are not required
- [x] Documentation was added or is not required



Patch:
diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go
index 1ac8fb25a3..3e37c3cf1e 100644
--- a/go/vt/vtgate/planbuilder/horizon_planning.go
+++ b/go/vt/vtgate/planbuilder/horizon_planning.go
@@ -459,7 +459,7 @@ func checkIfAlreadyExists(expr *sqlparser.AliasedExpr, node sqlparser.SelectStat
 func (hp *horizonPlanning) planAggregations(ctx *plancontext.PlanningContext, plan logicalPlan) (logicalPlan, error) {
 	newPlan := plan
 	var oa *orderedAggregate
-	uniqVindex := hasUniqueVindex(ctx.VSchema, ctx.SemTable, hp.qp.GroupByExprs)
+	uniqVindex := hasUniqueVindex(ctx.SemTable, hp.qp.GroupByExprs)
 	joinPlan := isJoin(plan)
 	if !uniqVindex || joinPlan {
 		if hp.qp.ProjectionError != nil {
@@ -622,9 +622,9 @@ func (hp *horizonPlanning) createPushExprAndAlias(
 	return aliasExpr, param
 }
 
-func hasUniqueVindex(vschema plancontext.VSchema, semTable *semantics.SemTable, groupByExprs []abstract.GroupBy) bool {
+func hasUniqueVindex(semTable *semantics.SemTable, groupByExprs []abstract.GroupBy) bool {
 	for _, groupByExpr := range groupByExprs {
-		if exprHasUniqueVindex(vschema, semTable, groupByExpr.WeightStrExpr) {
+		if exprHasUniqueVindex(semTable, groupByExpr.WeightStrExpr) {
 			return true
 		}
 	}
@@ -1018,7 +1018,7 @@ func (hp *horizonPlanning) planDistinct(ctx *plancontext.PlanningContext, plan l
 		// 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(ctx.VSchema, ctx.SemTable, hp.qp.SelectExprs) {
+		if p.isSingleShard() || selectHasUniqueVindex(ctx.SemTable, hp.qp.SelectExprs) {
 			return plan, nil
 		}
 
@@ -1147,14 +1147,14 @@ func isAmbiguousOrderBy(index int, col sqlparser.ColIdent, exprs []abstract.Sele
 	return false
 }
 
-func selectHasUniqueVindex(vschema plancontext.VSchema, semTable *semantics.SemTable, sel []abstract.SelectExpr) bool {
+func selectHasUniqueVindex(semTable *semantics.SemTable, sel []abstract.SelectExpr) bool {
 	for _, expr := range sel {
 		exp, err := expr.GetExpr()
 		if err != nil {
 			// TODO: handle star expression error
 			return false
 		}
-		if exprHasUniqueVindex(vschema, semTable, exp) {
+		if exprHasUniqueVindex(semTable, exp) {
 			return true
 		}
 	}
@@ -1185,7 +1185,7 @@ func (hp *horizonPlanning) needDistinctHandling(
 		// Unreachable
 		return true, innerAliased, nil
 	}
-	if exprHasUniqueVindex(ctx.VSchema, ctx.SemTable, innerAliased.Expr) {
+	if exprHasUniqueVindex(ctx.SemTable, innerAliased.Expr) {
 		// if we can see a unique vindex on this table/column,
 		// we know the results will be unique, and we don't need to DISTINCTify them
 		return false, nil, nil
@@ -1225,7 +1225,11 @@ func isJoin(plan logicalPlan) bool {
 	}
 }
 
-func exprHasUniqueVindex(vschema plancontext.VSchema, semTable *semantics.SemTable, expr sqlparser.Expr) bool {
+func exprHasUniqueVindex(semTable *semantics.SemTable, expr sqlparser.Expr) bool {
+	return exprHasVindex(semTable, expr, true)
+}
+
+func exprHasVindex(semTable *semantics.SemTable, expr sqlparser.Expr, hasToBeUnique bool) bool {
 	col, isCol := expr.(*sqlparser.ColName)
 	if !isCol {
 		return false
@@ -1235,16 +1239,9 @@ func exprHasUniqueVindex(vschema plancontext.VSchema, semTable *semantics.SemTab
 	if err != nil {
 		return false
 	}
-	tableName, err := tableInfo.Name()
-	if err != nil {
-		return false
-	}
-	vschemaTable, _, _, _, _, err := vschema.FindTableOrVindex(tableName)
-	if err != nil {
-		return false
-	}
+	vschemaTable := tableInfo.GetVindexTable()
 	for _, vindex := range vschemaTable.ColumnVindexes {
-		if len(vindex.Columns) > 1 || !vindex.IsUnique() {
+		if len(vindex.Columns) > 1 || hasToBeUnique && !vindex.IsUnique() {
 			return false
 		}
 		if col.Name.Equal(vindex.Columns[0]) {


Test patch:
diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.txt b/go/vt/vtgate/planbuilder/testdata/select_cases.txt
index 331e0765b1..648c17eb94 100644
--- a/go/vt/vtgate/planbuilder/testdata/select_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/select_cases.txt
@@ -3039,3 +3039,22 @@ Gen4 plan same as above
     "Table": "dual"
   }
 }
+
+# groupe by with non aggregated columns and table alias
+"select u.id, u.age from user u group by u.id"
+{
+  "QueryType": "SELECT",
+  "Original": "select u.id, u.age from user u group by u.id",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "Scatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select u.id, u.age from `user` as u where 1 != 1 group by u.id",
+    "Query": "select u.id, u.age from `user` as u group by u.id",
+    "Table": "`user`"
+  }
+}
+Gen4 plan same as above