[13.0] Cherry Pick: only use the special NoRoute handling mode when n…

…eeded (#11020)

* fix V3: only use the special NoRoute handling mode when needed (#10422)

Signed-off-by: Andres Taylor <andres@planetscale.com>

* fix: build

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

* Fix failure when Unowned lookup IS NULL (#10346)

* Add a failing test case showing Gen4 planner failure.

This test case shows a regression in the query planner for queries that use `IS NULL` condition on an unowned lookup vindex column.

Signed-off-by: Arthur Schreiber <arthurschreiber@github.com>
Co-authored-by: Elisa Vaccaro <30807261+evaccaro@users.noreply.github.com>
Co-authored-by: Jakob Class <mikrobi@github.com>

* bugfix: Limit the use of NoRoutesSpecialHandling to when it's needed

Signed-off-by: Andres Taylor <andres@planetscale.com>

Co-authored-by: Arthur Schreiber <arthurschreiber@github.com>
Co-authored-by: Elisa Vaccaro <30807261+evaccaro@users.noreply.github.com>
Co-authored-by: Jakob Class <mikrobi@github.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>

Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Co-authored-by: Andres Taylor <andres@planetscale.com>
Co-authored-by: Arthur Schreiber <arthurschreiber@github.com>
Co-authored-by: Elisa Vaccaro <30807261+evaccaro@users.noreply.github.com>
Co-authored-by: Jakob Class <mikrobi@github.com>
Co-authored-by: deepthi <deepthi@planetscale.com>

go/test/endtoend/vtgate/lookup_test.go

@@ -77,6 +77,20 @@ func TestUnownedLookupInsertChecksKeyspaceIdsAreMatching(t *testing.T) {
 	utils.Exec(t, conn, "delete from t9 WHERE parent_id = 1")
 }
 
+func TestUnownedLookupSelectNull(t *testing.T) {
+	defer cluster.PanicHandler(t)
+
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.Nil(t, err)
+	defer conn.Close()
+
+	utils.Exec(t, conn, "select * from t8 WHERE t9_id IS NULL")
+
+	// Cleanup
+	utils.Exec(t, conn, "delete from t9 WHERE parent_id = 1")
+}
+
 func TestConsistentLookup(t *testing.T) {
 	defer cluster.PanicHandler(t)
 	ctx := context.Background()

go/test/endtoend/vtgate/sec_vind/main_test.go

@@ -0,0 +1,112 @@
+/*
+Copyright 2022 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package vtgate
+
+import (
+	"context"
+	_ "embed"
+	"flag"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/test/endtoend/cluster"
+	"vitess.io/vitess/go/test/endtoend/vtgate/utils"
+)
+
+var (
+	clusterInstance *cluster.LocalProcessCluster
+	vtParams        mysql.ConnParams
+	KeyspaceName    = "ks"
+	Cell            = "test"
+	//go:embed schema.sql
+	SchemaSQL string
+
+	//go:embed vschema.json
+	VSchema string
+)
+
+func TestMain(m *testing.M) {
+	defer cluster.PanicHandler(nil)
+	flag.Parse()
+
+	exitCode := func() int {
+		clusterInstance = cluster.NewCluster(Cell, "localhost")
+		defer clusterInstance.Teardown()
+
+		// Start topo server
+		err := clusterInstance.StartTopo()
+		if err != nil {
+			return 1
+		}
+
+		// Start keyspace
+		keyspace := &cluster.Keyspace{
+			Name:      KeyspaceName,
+			SchemaSQL: SchemaSQL,
+			VSchema:   VSchema,
+		}
+		err = clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 1, true)
+		if err != nil {
+			return 1
+		}
+
+		// Start vtgate
+		err = clusterInstance.StartVtgate()
+		if err != nil {
+			return 1
+		}
+		vtParams = mysql.ConnParams{
+			Host: clusterInstance.Hostname,
+			Port: clusterInstance.VtgateMySQLPort,
+		}
+		return m.Run()
+	}()
+	os.Exit(exitCode)
+}
+
+func start(t *testing.T) (*mysql.Conn, func()) {
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.Nil(t, err)
+
+	deleteAll := func() {
+		_, _ = utils.ExecAllowError(t, conn, "set workload = oltp")
+
+		tables := []string{"t1", "lookup_t1"}
+		for _, table := range tables {
+			_, _ = utils.ExecAllowError(t, conn, "delete from "+table)
+		}
+	}
+
+	deleteAll()
+
+	return conn, func() {
+		deleteAll()
+		conn.Close()
+		cluster.PanicHandler(t)
+	}
+}
+
+func TestInAgainstSecondaryVindex(t *testing.T) {
+	conn, closer := start(t)
+	defer closer()
+
+	utils.AssertMatches(t, conn, `select 1 from t1 where c2 in ("abc")`, "[]")
+}

go/test/endtoend/vtgate/sec_vind/schema.sql

@@ -0,0 +1,11 @@
+CREATE TABLE t1 (
+                    c1 bigint unsigned NOT NULL,
+                    c2 varchar(16) NOT NULL,
+                    PRIMARY KEY (c1)
+) ENGINE InnoDB;
+
+CREATE TABLE lookup_t1 (
+                           c2 varchar(16) NOT NULL,
+                           keyspace_id varbinary(16) NOT NULL,
+                           PRIMARY KEY (c2)
+) ENGINE InnoDB;

go/test/endtoend/vtgate/sec_vind/vschema.json

@@ -0,0 +1,39 @@
+{
+  "sharded": true,
+  "vindexes": {
+    "lookup_idx": {
+      "type": "lookup",
+      "params": {
+        "from": "c2",
+        "to": "keyspace_id",
+        "table": "lookup_t1"
+      },
+      "owner": "t1"
+    },
+    "xxhash": {
+      "type": "xxhash"
+    }
+  },
+  "tables": {
+    "t1": {
+      "columnVindexes": [
+        {
+          "column": "c1",
+          "name": "xxhash"
+        },
+        {
+          "column": "c2",
+          "name": "lookup_idx"
+        }
+      ]
+    },
+    "lookup_t1": {
+      "columnVindexes": [
+        {
+          "column": "c2",
+          "name": "xxhash"
+        }
+      ]
+    }
+  }
+}

go/vt/sqlparser/ast_funcs.go

@@ -1569,6 +1569,29 @@ func (es *ExtractedSubquery) updateAlternative() {
 	}
 }
 
+// ColumnName returns the alias if one was provided, otherwise prints the AST
+func (ae *AliasedExpr) ColumnName() string {
+	if !ae.As.IsEmpty() {
+		return ae.As.String()
+	}
+
+	if col, ok := ae.Expr.(*ColName); ok {
+		return col.Name.String()
+	}
+
+	return String(ae.Expr)
+}
+
+// AllAggregation returns true if all the expressions contain aggregation
+func (s SelectExprs) AllAggregation() bool {
+	for _, k := range s {
+		if !ContainsAggregation(k) {
+			return false
+		}
+	}
+	return true
+}
+
 func isExprLiteral(expr Expr) bool {
 	switch expr := expr.(type) {
 	case *Literal:

go/vt/sqlparser/ast_rewriting.go

@@ -568,6 +568,34 @@ func (er *expressionRewriter) unnestSubQueries(cursor *Cursor, subquery *Subquer
 	cursor.Replace(rewritten)
 }
 
+func (er *expressionRewriter) existsRewrite(cursor *Cursor, node *ExistsExpr) {
+	switch node := node.Subquery.Select.(type) {
+	case *Select:
+		if node.Limit == nil {
+			node.Limit = &Limit{}
+		}
+		node.Limit.Rowcount = NewIntLiteral("1")
+
+		if node.Having != nil {
+			// If the query has HAVING, we can't take any shortcuts
+			return
+		}
+
+		if len(node.GroupBy) == 0 && node.SelectExprs.AllAggregation() {
+			// in these situations, we are guaranteed to always get a non-empty result,
+			// so we can replace the EXISTS with a literal true
+			cursor.Replace(BoolVal(true))
+		}
+
+		// If we are not doing HAVING, we can safely replace all select expressions with a
+		// single `1` and remove any grouping
+		node.SelectExprs = SelectExprs{
+			&AliasedExpr{Expr: NewIntLiteral("1")},
+		}
+		node.GroupBy = nil
+	}
+}
+
 func bindVarExpression(name string) Expr {
 	return NewArgument(name)
 }

go/vt/vtgate/executor_select_test.go

@@ -776,7 +776,7 @@ func TestSelectBindvars(t *testing.T) {
 	})
 	require.NoError(t, err)
 	wantQueries = []*querypb.BoundQuery{{
-		Sql: "select id from `user` where `name` in ::__vals",
+		Sql: "select id from `user` where 1 != 1",
 		BindVariables: map[string]*querypb.BindVariable{
 			"name1": sqltypes.BytesBindVariable([]byte("foo1")),
 			"name2": sqltypes.BytesBindVariable([]byte("foo2")),
@@ -807,7 +807,7 @@ func TestSelectBindvars(t *testing.T) {
 
 	// When there are no matching rows in the vindex, vtgate still needs the field info
 	wantQueries = []*querypb.BoundQuery{{
-		Sql: "select id from `user` where `name` = :name",
+		Sql: "select id from `user` where 1 != 1",
 		BindVariables: map[string]*querypb.BindVariable{
 			"name": sqltypes.StringBindVariable("nonexistent"),
 		},

go/vt/vtgate/planbuilder/gen4_planner.go

@@ -80,12 +80,14 @@ func gen4Planner(query string, plannerVersion querypb.ExecuteOptions_PlannerVers
 		}
 
 		primitive := plan.Primitive()
-		if rb, ok := primitive.(*engine.Route); ok {
+		if rb, ok := primitive.(*engine.Route); ok && isSel {
 			// this is done because engine.Route doesn't handle the empty result well
 			// if it doesn't find a shard to send the query to.
 			// All other engine primitives can handle this, so we only need it when
 			// Route is the last (and only) instruction before the user sees a result
-			rb.NoRoutesSpecialHandling = true
+			if isOnlyDual(sel) || (len(sel.GroupBy) == 0 && sel.SelectExprs.AllAggregation()) {
+				rb.NoRoutesSpecialHandling = true
+			}
 		}
 
 		return primitive, nil

go/vt/vtgate/planbuilder/select.go

@@ -79,7 +79,9 @@ func buildSelectPlan(query string) selectPlanner {
 			// if it doesn't find a shard to send the query to.
 			// All other engine primitives can handle this, so we only need it when
 			// Route is the last (and only) instruction before the user sees a result
-			rb.NoRoutesSpecialHandling = true
+			if isOnlyDual(sel) || (len(sel.GroupBy) == 0 && sel.SelectExprs.AllAggregation()) {
+				rb.NoRoutesSpecialHandling = true
+			}
 		}
 
 		return primitive, nil

test/config.json

@@ -993,6 +993,15 @@
 			"RetryMax": 2,
 			"Tags": []
 		},
+		"vindex_secondary": {
+			"File": "unused.go",
+			"Args": ["vitess.io/vitess/go/test/endtoend/vtgate/sec_vind"],
+			"Command": [],
+			"Manual": false,
+			"Shard": "vtgate_vindex",
+			"RetryMax": 2,
+			"Tags": []
+		},
 		"web_test": {
 			"File": "unused.go",
 			"Args": ["vitess.io/vitess/go/test/endtoend/vtctldweb"],