Repository: vitessio/vitess
Pull request #6133 (2020-04-30 08:35:47)

Create better plans for NULL comparisons


Patch:
diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go
index c6b098779d..0e799d93c2 100644
--- a/go/vt/vtgate/engine/route.go
+++ b/go/vt/vtgate/engine/route.go
@@ -161,6 +161,8 @@ const (
 	SelectDBA
 	// SelectReference is for fetching from a reference table.
 	SelectReference
+	// SelectNone is used for queries that always return empty values
+	SelectNone
 )
 
 var routeName = map[RouteOpcode]string{
@@ -172,6 +174,7 @@ var routeName = map[RouteOpcode]string{
 	SelectNext:        "SelectNext",
 	SelectDBA:         "SelectDBA",
 	SelectReference:   "SelectReference",
+	SelectNone:        "SelectNone",
 }
 
 var (
@@ -230,6 +233,8 @@ func (route *Route) execute(vcursor VCursor, bindVars map[string]*querypb.BindVa
 		rss, bvs, err = route.paramsSelectEqual(vcursor, bindVars)
 	case SelectIN:
 		rss, bvs, err = route.paramsSelectIn(vcursor, bindVars)
+	case SelectNone:
+		rss, bvs, err = nil, nil, nil
 	default:
 		// Unreachable.
 		return nil, fmt.Errorf("unsupported query route: %v", route)
diff --git a/go/vt/vtgate/planbuilder/route_option.go b/go/vt/vtgate/planbuilder/route_option.go
index 787254a813..e374e03ad6 100644
--- a/go/vt/vtgate/planbuilder/route_option.go
+++ b/go/vt/vtgate/planbuilder/route_option.go
@@ -199,13 +199,19 @@ func (ro *routeOption) canMergeOnFilter(pb *primitiveBuilder, rro *routeOption,
 // the route.
 func (ro *routeOption) UpdatePlan(pb *primitiveBuilder, filter sqlparser.Expr) {
 	switch ro.eroute.Opcode {
-	case engine.SelectUnsharded, engine.SelectNext, engine.SelectDBA, engine.SelectReference:
+	// For these opcodes, a new filter will not make any difference, so we can just exit early
+	case engine.SelectUnsharded, engine.SelectNext, engine.SelectDBA, engine.SelectReference, engine.SelectNone:
 		return
 	}
 	opcode, vindex, values := ro.computePlan(pb, filter)
 	if opcode == engine.SelectScatter {
 		return
 	}
+	// If we get SelectNone in next filters, override the previous route plan.
+	if opcode == engine.SelectNone {
+		ro.updateRoute(opcode, vindex, values)
+		return
+	}
 	switch ro.eroute.Opcode {
 	case engine.SelectEqualUnique:
 		if opcode == engine.SelectEqualUnique && vindex.Cost() < ro.eroute.Vindex.Cost() {
@@ -231,7 +237,7 @@ func (ro *routeOption) UpdatePlan(pb *primitiveBuilder, filter sqlparser.Expr) {
 		}
 	case engine.SelectScatter:
 		switch opcode {
-		case engine.SelectEqualUnique, engine.SelectEqual, engine.SelectIN:
+		case engine.SelectEqualUnique, engine.SelectEqual, engine.SelectIN, engine.SelectNone:
 			ro.updateRoute(opcode, vindex, values)
 		}
 	}
@@ -253,6 +259,8 @@ func (ro *routeOption) computePlan(pb *primitiveBuilder, filter sqlparser.Expr)
 		case sqlparser.InStr:
 			return ro.computeINPlan(pb, node)
 		}
+	case *sqlparser.IsExpr:
+		return ro.computeISPlan(pb, node)
 	}
 	return engine.SelectScatter, nil, nil
 }
@@ -269,6 +277,9 @@ func (ro *routeOption) computeEqualPlan(pb *primitiveBuilder, comparison *sqlpar
 			return engine.SelectScatter, nil, nil
 		}
 	}
+	if sqlparser.IsNull(right) {
+		return engine.SelectNone, nil, nil
+	}
 	if !ro.exprIsValue(right) {
 		return engine.SelectScatter, nil, nil
 	}
@@ -278,6 +289,23 @@ func (ro *routeOption) computeEqualPlan(pb *primitiveBuilder, comparison *sqlpar
 	return engine.SelectEqual, vindex, right
 }
 
+// computeEqualPlan computes the plan for an equality constraint.
+func (ro *routeOption) computeISPlan(pb *primitiveBuilder, comparison *sqlparser.IsExpr) (opcode engine.RouteOpcode, vindex vindexes.SingleColumn, condition sqlparser.Expr) {
+	// we only handle IS NULL correct. IsExpr can contain other expressions as well
+	if comparison.Operator != sqlparser.IsNullStr {
+		return engine.SelectScatter, nil, nil
+	}
+
+	vindex = ro.FindVindex(pb, comparison.Expr)
+	if vindex == nil {
+		return engine.SelectScatter, nil, nil
+	}
+	if vindex.IsUnique() {
+		return engine.SelectEqualUnique, vindex, &sqlparser.NullVal{}
+	}
+	return engine.SelectEqual, vindex, &sqlparser.NullVal{}
+}
+
 // computeINPlan computes the plan for an IN constraint.
 func (ro *routeOption) computeINPlan(pb *primitiveBuilder, comparison *sqlparser.ComparisonExpr) (opcode engine.RouteOpcode, vindex vindexes.SingleColumn, condition sqlparser.Expr) {
 	vindex = ro.FindVindex(pb, comparison.Left)


Test patch:
diff --git a/go/test/endtoend/vtgate/lookup_test.go b/go/test/endtoend/vtgate/lookup_test.go
index a5b485399c..598c814582 100644
--- a/go/test/endtoend/vtgate/lookup_test.go
+++ b/go/test/endtoend/vtgate/lookup_test.go
@@ -22,6 +22,8 @@ import (
 	"strings"
 	"testing"
 
+	"vitess.io/vitess/go/test/utils"
+
 	"github.com/stretchr/testify/require"
 
 	"vitess.io/vitess/go/mysql"
@@ -503,6 +505,30 @@ func TestConsistentLookupUpdate(t *testing.T) {
 	require.Empty(t, qr.Rows)
 }
 
+func TestSelectNull(t *testing.T) {
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	exec(t, conn, "begin")
+	exec(t, conn, "insert into t5_null_vindex(id, idx) values(1, 'a'), (2, 'b'), (3, null)")
+	exec(t, conn, "commit")
+	qr := exec(t, conn, "select id, idx from t5_null_vindex order by id")
+	utils.MustMatch(t, fmt.Sprintf("%v", qr.Rows), "[[INT64(1) VARCHAR(\"a\")] [INT64(2) VARCHAR(\"b\")] [INT64(3) NULL]]", "")
+
+	qr = exec(t, conn, "select id, idx from t5_null_vindex where idx = null")
+	require.Empty(t, qr.Rows)
+
+	qr = exec(t, conn, "select id, idx from t5_null_vindex where idx is null")
+	utils.MustMatch(t, fmt.Sprintf("%v", qr.Rows), "[[INT64(3) NULL]]", "")
+
+	qr = exec(t, conn, "select id, idx from t5_null_vindex where idx is not null order by id")
+	utils.MustMatch(t, fmt.Sprintf("%v", qr.Rows), "[[INT64(1) VARCHAR(\"a\")] [INT64(2) VARCHAR(\"b\")]]", "")
+
+	exec(t, conn, "delete from t5_null_vindex")
+}
+
 func exec(t *testing.T, conn *mysql.Conn, query string) *sqltypes.Result {
 	t.Helper()
 	qr, err := conn.ExecuteFetch(query, 1000, true)
diff --git a/go/test/endtoend/vtgate/main_test.go b/go/test/endtoend/vtgate/main_test.go
index 5ca001fb8a..c23de26605 100644
--- a/go/test/endtoend/vtgate/main_test.go
+++ b/go/test/endtoend/vtgate/main_test.go
@@ -94,6 +94,12 @@ create table t4_id2_idx(
 	id1 bigint,
 	keyspace_id varbinary(50),
     primary key(id2, id1)
+) Engine=InnoDB;
+
+create table t5_null_vindex(
+	id bigint not null,
+	idx varchar(50),
+	primary key(id)
 ) Engine=InnoDB;`
 
 	VSchema = `
@@ -106,6 +112,9 @@ create table t4_id2_idx(
     "hash": {
       "type": "hash"
     },
+    "xxhash": {
+      "type": "xxhash"
+    },
     "t1_id2_vdx": {
       "type": "consistent_lookup_unique",
       "params": {
@@ -224,6 +233,14 @@ create table t4_id2_idx(
           "name": "unicode_loose_md5"
         }
       ]
+    },
+	"t5_null_vindex": {
+      "column_vindexes": [
+        {
+          "column": "idx",
+          "name": "xxhash"
+        }
+      ]
     },
     "vstream_test": {
       "column_vindexes": [
diff --git a/go/vt/vtgate/engine/route_test.go b/go/vt/vtgate/engine/route_test.go
index 5855e2e587..41ae7d2335 100644
--- a/go/vt/vtgate/engine/route_test.go
+++ b/go/vt/vtgate/engine/route_test.go
@@ -20,6 +20,8 @@ import (
 	"errors"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"vitess.io/vitess/go/mysql"
 	"vitess.io/vitess/go/sqltypes"
 	querypb "vitess.io/vitess/go/vt/proto/query"
@@ -148,6 +150,40 @@ func TestSelectEqualUnique(t *testing.T) {
 	expectResult(t, "sel.StreamExecute", result, defaultSelectResult)
 }
 
+func TestSelectNone(t *testing.T) {
+	vindex, _ := vindexes.NewHash("", nil)
+	sel := NewRoute(
+		SelectNone,
+		&vindexes.Keyspace{
+			Name:    "ks",
+			Sharded: true,
+		},
+		"dummy_select",
+		"dummy_select_field",
+	)
+	sel.Vindex = vindex.(vindexes.SingleColumn)
+	sel.Values = nil
+
+	vc := &loggingVCursor{
+		shards:  []string{"-20", "20-"},
+		results: []*sqltypes.Result{},
+	}
+	result, err := sel.Execute(vc, map[string]*querypb.BindVariable{}, false)
+	require.NoError(t, err)
+	require.Empty(t, vc.log)
+	expectResult(t, "sel.Execute", result, &sqltypes.Result{})
+
+	vc.Rewind()
+
+	result, err = sel.Execute(vc, map[string]*querypb.BindVariable{}, true)
+	require.NoError(t, err)
+	vc.ExpectLog(t, []string{
+		`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
+		`ExecuteMultiShard ks.-20: dummy_select_field {} false false`,
+	})
+	expectResult(t, "sel.Execute", result, &sqltypes.Result{})
+}
+
 func TestSelectEqualUniqueScatter(t *testing.T) {
 	vindex, _ := vindexes.NewLookupUnique("", map[string]string{
 		"table":      "lkp",
diff --git a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt
index 9216bc929f..8bb634c68e 100644
--- a/go/vt/vtgate/planbuilder/testdata/filter_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/filter_cases.txt
@@ -1257,3 +1257,79 @@
 # and the second reference is to the innermost 'from' subquery.
 "select id2 from user uu where id in (select id from user where id = uu.id and user.col in (select col from (select id from user_extra where user_id = 5) uu where uu.user_id = uu.id))"
 "unsupported: cross-shard correlated subquery"
+
+# Select with equals null
+"select id from music where id = null"
+{
+  "QueryType": "SELECT",
+  "Original": "select id from music where id = null",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectNone",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select id from music where 1 != 1",
+    "Query": "select id from music where id = null",
+    "Table": "music"
+  }
+}
+
+# SELECT with IS NULL
+"select id from music where id is null"
+{
+  "QueryType": "SELECT",
+  "Original": "select id from music where id is null",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectEqualUnique",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select id from music where 1 != 1",
+    "Query": "select id from music where id is null",
+    "Table": "music",
+    "Values": [
+      null
+    ],
+    "Vindex": "music_user_map"
+  }
+}
+
+# SELECT with IS NOT NULL
+"select id from music where id is not null"
+{
+  "QueryType": "SELECT",
+  "Original": "select id from music where id is not null",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectScatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select id from music where 1 != 1",
+    "Query": "select id from music where id is not null",
+    "Table": "music"
+  }
+}
+
+# Single table with unique vindex match and null match
+"select id from music where user_id = 4 and id = null"
+{
+  "QueryType": "SELECT",
+  "Original": "select id from music where user_id = 4 and id = null",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "SelectNone",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select id from music where 1 != 1",
+    "Query": "select id from music where user_id = 4 and id = null",
+    "Table": "music"
+  }
+}