added planning support for group_concat

Signed-off-by: Harshit Gangal <[email protected]>
vitessio · Jun 20, 2023 · 08285da · 08285da
1 parent ccb75a5
commit 08285da
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 11 deletions.
diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go
@@ -20,17 +20,14 @@ import (
 	"fmt"
 
 	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/vt/sqlparser"
+	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/engine"
 	popcode "vitess.io/vitess/go/vt/vtgate/engine/opcode"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/operators"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
-
 	"vitess.io/vitess/go/vt/vtgate/semantics"
-
-	"vitess.io/vitess/go/vt/vterrors"
-
-	"vitess.io/vitess/go/vt/sqlparser"
-	"vitess.io/vitess/go/vt/vtgate/engine"
 )
 
 type horizonPlanning struct {
@@ -316,6 +313,10 @@ func (hp *horizonPlanning) planAggrUsingOA(
 		oa.preProcess = true
 	}
 
+	if err = unsupportedAggregations(aggrs); err != nil {
+		return nil, err
+	}
+
 	newPlan, groupingOffsets, aggrParamOffsets, pushed, err := hp.pushAggregation(ctx, plan, grouping, aggrs, false)
 	if err != nil {
 		return nil, err
@@ -369,6 +370,15 @@ func (hp *horizonPlanning) planAggrUsingOA(
 	return hp.planHaving(ctx, oa)
 }
 
+func unsupportedAggregations(aggrs []operators.Aggr) error {
+	for _, aggr := range aggrs {
+		if aggr.OpCode == popcode.AggregateGroupConcat {
+			return vterrors.VT12001(fmt.Sprintf("in scatter query: aggregation function '%s'", sqlparser.String(aggr.Func)))
+		}
+	}
+	return nil
+}
+
 func passGroupingColumns(proj *projection, groupings []offsets, grouping []operators.GroupBy) (projGrpOffsets []offsets, err error) {
 	for idx, grp := range groupings {
 		origGrp := grouping[idx]

diff --git a/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go b/go/vt/vtgate/planbuilder/operators/aggregation_pushing.go
@@ -466,7 +466,10 @@ func (ab *aggBuilder) handleAggr(ctx *plancontext.PlanningContext, aggr Aggr) er
 		if f.Distinct || len(f.OrderBy) > 0 || f.Separator != "" {
 			panic("fail here")
 		}
-		return ab.handlePushThroughAggregation(ctx, aggr)
+		// this needs special handling, currently aborting the push of function
+		// and later will try pushing the column instead.
+		// TODO: this should be handled better by pushing the function down.
+		return errAbortAggrPushing
 	case opcode.AggregateUnassigned:
 		return vterrors.VT12001(fmt.Sprintf("in scatter query: aggregation function '%s'", sqlparser.String(aggr.Original)))
 	default:

diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json
@@ -6820,8 +6820,8 @@
                   "Name": "user",
                   "Sharded": true
                 },
-                "FieldQuery": "select group_concat(`user`.a) from `user` where 1 != 1",
-                "Query": "select group_concat(`user`.a) from `user`",
+                "FieldQuery": "select `user`.a from `user` where 1 != 1",
+                "Query": "select `user`.a from `user`",
                 "Table": "`user`"
               },
               {
@@ -6831,8 +6831,8 @@
                   "Name": "user",
                   "Sharded": true
                 },
-                "FieldQuery": "select 1 from user_extra where 1 != 1 group by .0",
-                "Query": "select 1 from user_extra group by .0",
+                "FieldQuery": "select 1 from user_extra where 1 != 1",
+                "Query": "select 1 from user_extra",
                 "Table": "user_extra"
               }
             ]
@@ -6844,5 +6844,126 @@
         "user.user_extra"
       ]
     }
+  },
+  {
+    "comment": "group_concat with group by without in select list",
+    "query": "select group_concat(user.id) from user, music where user.id = music.foo group by user.bar",
+    "v3-plan": "VT12001: unsupported: cross-shard query with aggregates",
+    "gen4-plan": {
+      "QueryType": "SELECT",
+      "Original": "select group_concat(user.id) from user, music where user.id = music.foo group by user.bar",
+      "Instructions": {
+        "OperatorType": "Aggregate",
+        "Variant": "Ordered",
+        "Aggregates": "group_concat(0) AS group_concat(`user`.id)",
+        "GroupBy": "(1|2)",
+        "ResultColumns": 1,
+        "Inputs": [
+          {
+            "OperatorType": "Sort",
+            "Variant": "Memory",
+            "OrderBy": "(1|2) ASC",
+            "Inputs": [
+              {
+                "OperatorType": "Join",
+                "Variant": "Join",
+                "JoinColumnIndexes": "R:0,R:1,R:2",
+                "JoinVars": {
+                  "music_foo": 0
+                },
+                "TableName": "music_`user`",
+                "Inputs": [
+                  {
+                    "OperatorType": "Route",
+                    "Variant": "Scatter",
+                    "Keyspace": {
+                      "Name": "user",
+                      "Sharded": true
+                    },
+                    "FieldQuery": "select music.foo from music where 1 != 1",
+                    "Query": "select music.foo from music",
+                    "Table": "music"
+                  },
+                  {
+                    "OperatorType": "Route",
+                    "Variant": "EqualUnique",
+                    "Keyspace": {
+                      "Name": "user",
+                      "Sharded": true
+                    },
+                    "FieldQuery": "select `user`.id, `user`.bar, weight_string(`user`.bar) from `user` where 1 != 1",
+                    "Query": "select `user`.id, `user`.bar, weight_string(`user`.bar) from `user` where `user`.id = :music_foo",
+                    "Table": "`user`",
+                    "Values": [
+                      ":music_foo"
+                    ],
+                    "Vindex": "user_index"
+                  }
+                ]
+              }
+            ]
+          }
+        ]
+      },
+      "TablesUsed": [
+        "user.music",
+        "user.user"
+      ]
+    }
+  },
+  {
+    "comment": "group_concat aggregation on top of route",
+    "query": "select intcol, group_concat(foo) from user group by intcol",
+    "v3-plan": {
+      "QueryType": "SELECT",
+      "Original": "select intcol, group_concat(foo) from user group by intcol",
+      "Instructions": {
+        "OperatorType": "Aggregate",
+        "Variant": "Ordered",
+        "Aggregates": "group_concat(1)",
+        "GroupBy": "0",
+        "Inputs": [
+          {
+            "OperatorType": "Route",
+            "Variant": "Scatter",
+            "Keyspace": {
+              "Name": "user",
+              "Sharded": true
+            },
+            "FieldQuery": "select intcol, group_concat(foo) from `user` where 1 != 1 group by intcol",
+            "OrderBy": "0 ASC",
+            "Query": "select intcol, group_concat(foo) from `user` group by intcol order by intcol asc",
+            "Table": "`user`"
+          }
+        ]
+      }
+    },
+    "gen4-plan": {
+      "QueryType": "SELECT",
+      "Original": "select intcol, group_concat(foo) from user group by intcol",
+      "Instructions": {
+        "OperatorType": "Aggregate",
+        "Variant": "Ordered",
+        "Aggregates": "group_concat(1) AS group_concat(foo)",
+        "GroupBy": "0",
+        "Inputs": [
+          {
+            "OperatorType": "Route",
+            "Variant": "Scatter",
+            "Keyspace": {
+              "Name": "user",
+              "Sharded": true
+            },
+            "FieldQuery": "select intcol, group_concat(foo) from `user` where 1 != 1 group by intcol",
+            "OrderBy": "0 ASC",
+            "Query": "select intcol, group_concat(foo) from `user` group by intcol order by intcol asc",
+            "Table": "`user`"
+          }
+        ]
+      },
+      "TablesUsed": [
+        "user.user"
+      ]
+    }
   }
 ]