bugfixes: collection of fixes to bugs found while fuzzing (vitessio#1…

…3332) * bugfix: left join predicate only depending on RHS turns into inner join Signed-off-by: Andres Taylor <andres@planetscale.com> * bugfix: use the incoming type fields to calculate the outgoing types Signed-off-by: Andres Taylor <andres@planetscale.com> * bugfix: filter was using wrong method on evalengine Signed-off-by: Andres Taylor <andres@planetscale.com> --------- Signed-off-by: Andres Taylor <andres@planetscale.com>
planetscale · Aug 17, 2023 · 6c34078 · 6c34078
1 parent 64fa0b1
commit 6c34078
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 5 deletions.
diff --git a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go
@@ -437,3 +437,20 @@ func TestAggregationRandomOnAnAggregatedValue(t *testing.T) {
 	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 t11 where a = 100) A;",
 		`[[DECIMAL(100) DECIMAL(10) DECIMAL(10.0000)]]`)
 }
+
+func TestBuggyQueries(t *testing.T) {
+	// These queries have been found to be producing the wrong results by the query fuzzer
+	// Adding them as end2end tests to make sure we never get them wrong again
+	mcmp, closer := start(t)
+	defer closer()
+
+	mcmp.Exec("insert into t10(k, a, b) values (0, 100, 10), (10, 200, 20), (20, null, null)")
+
+	mcmp.AssertMatches("select /*vt+ PLANNER=Gen4 */ sum(t1.a) from t10 as t1, t10 as t2",
+		`[[DECIMAL(900)]]`)
+
+	mcmp.AssertMatches("select /*vt+ PLANNER=gen4 */t1.a, sum(t1.a), count(*), t1.a, sum(t1.a), count(*) from t10 as t1, t10 as t2 group by t1.a",
+		"[[NULL NULL INT64(3) NULL NULL INT64(3)] "+
+			"[INT32(100) DECIMAL(300) INT64(3) INT32(100) DECIMAL(300) INT64(3)] "+
+			"[INT32(200) DECIMAL(600) INT64(3) INT32(200) DECIMAL(600) INT64(3)]]")
+}
diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go
@@ -160,3 +160,13 @@ func TestHighNumberOfParams(t *testing.T) {
 	}
 	require.Equal(t, 5, count)
 }
+
+func TestBuggyOuterJoin(t *testing.T) {
+	// We found a couple of inconsistencies around outer joins, adding these tests to stop regressions
+	mcmp, closer := start(t)
+	defer closer()
+
+	mcmp.Exec("insert into t1(id1, id2) values (1,2), (42,5), (5, 42)")
+
+	mcmp.Exec("select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2")
+}
diff --git a/go/vt/vtgate/engine/filter.go b/go/vt/vtgate/engine/filter.go
@@ -68,11 +68,8 @@ func (f *Filter) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[s
 		if err != nil {
 			return nil, err
 		}
-		intEvalResult, err := evalResult.Value().ToInt64()
-		if err != nil {
-			return nil, err
-		}
-		if intEvalResult == 1 {
+
+		if evalResult.ToBoolean() {
 			rows = append(rows, row)
 		}
 	}

diff --git a/go/vt/vtgate/evalengine/eval_result.go b/go/vt/vtgate/evalengine/eval_result.go
@@ -309,6 +309,10 @@ func (er *EvalResult) isTextual() bool {
 	return sqltypes.IsText(tt) || sqltypes.IsBinary(tt)
 }
 
+func (er *EvalResult) ToBoolean() bool {
+	return er.isTruthy() == boolTrue
+}
+
 func (er *EvalResult) isTruthy() boolean {
 	if er.isNull() {
 		return boolNULL

diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json
@@ -6287,5 +6287,83 @@
     "query": "select missing_column from unsharded, unsharded_tab",
     "v3-plan": "symbol missing_column not found",
     "gen4-plan": "Column 'missing_column' in field list is ambiguous"
+  },
+  {
+    "comment": "join predicate only depending on the RHS should not turn outer join into inner join",
+    "query": "select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2",
+    "v3-plan": {
+      "QueryType": "SELECT",
+      "Original": "select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2",
+      "Instructions": {
+        "OperatorType": "Join",
+        "Variant": "LeftJoin",
+        "JoinColumnIndexes": "L:0,R:0",
+        "TableName": "t1_t1",
+        "Inputs": [
+          {
+            "OperatorType": "Route",
+            "Variant": "Scatter",
+            "Keyspace": {
+              "Name": "zlookup_unique",
+              "Sharded": true
+            },
+            "FieldQuery": "select t1.id1 from t1 where 1 != 1",
+            "Query": "select t1.id1 from t1",
+            "Table": "t1"
+          },
+          {
+            "OperatorType": "Route",
+            "Variant": "Scatter",
+            "Keyspace": {
+              "Name": "zlookup_unique",
+              "Sharded": true
+            },
+            "FieldQuery": "select t2.id1 from t1 as t2 where 1 != 1",
+            "Query": "select t2.id1 from t1 as t2 where t2.id1 = t2.id2",
+            "Table": "t1"
+          }
+        ]
+      },
+      "TablesUsed": [
+        "zlookup_unique.t1"
+      ]
+    },
+    "gen4-plan": {
+      "QueryType": "SELECT",
+      "Original": "select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2",
+      "Instructions": {
+        "OperatorType": "Join",
+        "Variant": "LeftJoin",
+        "JoinColumnIndexes": "L:0,R:0",
+        "TableName": "t1_t1",
+        "Inputs": [
+          {
+            "OperatorType": "Route",
+            "Variant": "Scatter",
+            "Keyspace": {
+              "Name": "zlookup_unique",
+              "Sharded": true
+            },
+            "FieldQuery": "select t1.id1 from t1 where 1 != 1",
+            "Query": "select t1.id1 from t1",
+            "Table": "t1"
+          },
+          {
+            "OperatorType": "Route",
+            "Variant": "Scatter",
+            "Keyspace": {
+              "Name": "zlookup_unique",
+              "Sharded": true
+            },
+            "FieldQuery": "select t2.id1 from t1 as t2 where 1 != 1",
+            "Query": "select t2.id1 from t1 as t2 where t2.id1 = t2.id2",
+            "Table": "t1"
+          }
+        ]
+      },
+      "TablesUsed": [
+        "zlookup_unique.t1"
+      ]
+    }
   }
 ]