[gen4 planner] Make sure to not push down expressions when not possible

go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go

@@ -425,3 +425,13 @@ func TestScalarAggregate(t *testing.T) {
  mcmp.Exec("insert into aggr_test(id, val1, val2) values(1,'a',1), (2,'A',1), (3,'b',1), (4,'c',3), (5,'c',4)")
  mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */ count(distinct val1) from aggr_test", `[[INT64(3)]]`)
 }
+
+func TestAggregationRandomOnAnAggregatedValue(t *testing.T) {
+ mcmp, closer := start(t)
+ defer closer()
+
+ mcmp.Exec("insert into t10(k, a, b) values (0, 100, 10), (10, 200, 20);")
+
+ mcmp.AssertMatchesNoOrder("select /*vt+ PLANNER=gen4 */ A.a, A.b, (A.a / A.b) as d from (select sum(a) as a, sum(b) as b from t10 where a = 100) A;",
+ `[[DECIMAL(100) DECIMAL(10) DECIMAL(10.0000)]]`)
+}

go/test/endtoend/vtgate/queries/aggregation/schema.sql

@@ -71,3 +71,8 @@ CREATE TABLE t2 (
  PRIMARY KEY (id)
 ) ENGINE InnoDB;
 
+CREATE TABLE t10 (
+ k BIGINT PRIMARY KEY,
+ a INT,
+ b INT
+);

go/test/endtoend/vtgate/queries/aggregation/vschema.json

@@ -123,6 +123,14 @@
  "name": "hash"
  }
  ]
+ },
+ "t10": {
+ "column_vindexes": [
+ {
+ "column": "k",
+ "name": "hash"
+ }
+ ]
  }
  }
 }

go/vt/vtgate/engine/ordered_aggregate.go

@@ -526,7 +526,14 @@ func merge(
  val, _ := sqltypes.NewValue(sqltypes.VarBinary, data)
  result[aggr.Col] = val
  case AggregateRandom:
- // we just grab the first value per grouping. no need to do anything more complicated here
+ // we just grab the first value per grouping
+ // however, if the first row contains a Null value for this row we decide to ignore
+ // it and use the second row. there might some cases (i.e. `sum(a) / sum(b)`) on a sharded
+ // cluster for which MySQL will return Null on row1 and a value on row2. we want to return
+ // the computed value of row2.
+ if row1[aggr.Col].IsNull() {
+ result[aggr.Col] = row2[aggr.Col]
+ }
  default:
  return nil, nil, fmt.Errorf("BUG: Unexpected opcode: %v", aggr.Opcode)
  }

go/vt/vtgate/executor_select_test.go

@@ -3856,6 +3856,40 @@ func TestSelectAggregationData(t *testing.T) {
  }
 }
 
+func TestSelectAggregationRandom(t *testing.T) {
+ cell := "aa"
+ hc := discovery.NewFakeHealthCheck(nil)
+ createSandbox(KsTestSharded).VSchema = executorVSchema
+ getSandbox(KsTestUnsharded).VSchema = unshardedVSchema
+ serv := newSandboxForCells([]string{cell})
+ resolver := newTestResolver(hc, serv, cell)
+ shards := []string{"-20", "20-40", "40-60", "60-80", "80-a0", "a0-c0", "c0-e0", "e0-"}
+ var conns []*sandboxconn.SandboxConn
+ for _, shard := range shards {
+ sbc := hc.AddTestTablet(cell, shard, 1, KsTestSharded, shard, topodatapb.TabletType_PRIMARY, true, 1, nil)
+ conns = append(conns, sbc)
+
+ sbc.SetResults([]*sqltypes.Result{sqltypes.MakeTestResult(
+ sqltypes.MakeTestFields("a|b|c", "int64|int64|int64"),
+ "null|null|null",
+ )})
+ }
+
+ conns[0].SetResults([]*sqltypes.Result{sqltypes.MakeTestResult(
+ sqltypes.MakeTestFields("a|b|c", "int64|int64|int64"),
+ "10|1|10",
+ )})
+
+ executor := createExecutor(serv, cell, resolver)
+ executor.pv = querypb.ExecuteOptions_Gen4
+ session := NewAutocommitSession(&vtgatepb.Session{})
+
+ rs, err := executor.Execute(context.Background(), "TestSelectCFC", session,
+ "select /*vt+ PLANNER=gen4 */ A.a, A.b, (A.a / A.b) as c from (select sum(a) as a, sum(b) as b from user) A", nil)
+ require.NoError(t, err)
+ assert.Equal(t, `[[INT64(10) INT64(1) INT64(10)]]`, fmt.Sprintf("%v", rs.Rows))
+}
+
 func TestSelectHexAndBit(t *testing.T) {
  executor, _, _, _ := createExecutorEnv()
  executor.normalize = true