Repository: vitessio/vitess
Pull request #8087 (2021-05-09 16:38:12)

System schema query predicate fix
<!--
  Thank you for your contribution to the Vitess project.
  How to contribute: https://vitess.io/docs/contributing/
  Please first make sure there is an open Issue to discuss the feature/fix suggested in this PR.
  If this is a new feature, please mark the Issue as "RFC".
 -->

<!-- if this PR is Work in Progress please create it as a Draft Pull Request -->

## Description
<!-- A few sentences describing the overall goals of the pull request's commits. -->
<!-- If this is a bug fix and you think the fix should be backported, please write so. -->

## Related Issue(s)
<!-- List related issues and pull requests. If this PR fixes an issue, please add it using Fixes #????  -->

If the information_schema query contains a table and schema both and schema is actual db name different from keyspace name, then the query fails.

```
mysql> use commerce;
Database changed
mysql> select CONSTRAINT_NAME, TABLE_NAME from information_schema.table_constraints where constraint_schema = 'vt_commerce' and table_name = 't';
ERROR 1105 (HY000): keyspace vt_commerce not found in vschema
```

## Checklist
- [X] Tests were added or are not required
- [X] Documentation was added or is not required


Patch:
diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go
index 7c66da87c2..c8e2a40cdd 100644
--- a/go/vt/vtgate/engine/route.go
+++ b/go/vt/vtgate/engine/route.go
@@ -491,6 +491,11 @@ func (route *Route) routeInfoSchemaQuery(vcursor VCursor, bindVars map[string]*q
 	if tableName != "" {
 		rss, err := route.paramsRoutedTable(vcursor, bindVars, specifiedKS, tableName)
 		if err != nil {
+			// Only if keyspace is not found in vschema, we try with default keyspace.
+			// As the in the table_schema predicates for a keyspace 'ks' it can contain 'vt_ks'.
+			if vterrors.ErrState(err) == vterrors.BadDb {
+				return defaultRoute()
+			}
 			return nil, err
 		}
 		if rss != nil {
diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go
index 45872e3f21..fbf582fc21 100644
--- a/go/vt/vtgate/planbuilder/select.go
+++ b/go/vt/vtgate/planbuilder/select.go
@@ -159,24 +159,15 @@ func planAggregations(qp *queryProjection, plan logicalPlan, semTable *semantics
 func planOrderBy(qp *queryProjection, plan logicalPlan, semTable *semantics.SemTable) (logicalPlan, error) {
 	switch plan := plan.(type) {
 	case *route:
-		additionalColAdded := false
 		for _, order := range qp.orderExprs {
 			offset, exists := qp.orderExprColMap[order]
+			if !exists {
+				return nil, semantics.Gen4NotSupportedF("order by column not exists in select list")
+			}
 			colName, ok := order.Expr.(*sqlparser.ColName)
 			if !ok {
 				return nil, semantics.Gen4NotSupportedF("order by non-column expression")
 			}
-			if !exists {
-				expr := &sqlparser.AliasedExpr{
-					Expr: order.Expr,
-				}
-				var err error
-				offset, err = pushProjection(expr, plan, semTable)
-				if err != nil {
-					return nil, err
-				}
-				additionalColAdded = true
-			}
 
 			table := semTable.Dependencies(colName)
 			tableInfo, err := semTable.TableInfoFor(table)
@@ -209,7 +200,9 @@ func planOrderBy(qp *queryProjection, plan logicalPlan, semTable *semantics.SemT
 				if err != nil {
 					return nil, err
 				}
-				additionalColAdded = true
+			}
+			if weightStringOffset != -1 {
+				plan.eroute.TruncateColumnCount = len(qp.selectExprs) + len(qp.aggrExprs)
 			}
 
 			plan.eroute.OrderBy = append(plan.eroute.OrderBy, engine.OrderbyParams{
@@ -219,10 +212,6 @@ func planOrderBy(qp *queryProjection, plan logicalPlan, semTable *semantics.SemT
 			})
 			plan.Select.AddOrder(order)
 		}
-		if additionalColAdded {
-			plan.eroute.TruncateColumnCount = len(qp.selectExprs) + len(qp.aggrExprs)
-		}
-
 		return plan, nil
 	default:
 		return nil, semantics.Gen4NotSupportedF("ordering on complex query")
diff --git a/go/vt/vtgate/vindexes/vschema.go b/go/vt/vtgate/vindexes/vschema.go
index 3054b611dd..52d5af4be5 100644
--- a/go/vt/vtgate/vindexes/vschema.go
+++ b/go/vt/vtgate/vindexes/vschema.go
@@ -23,6 +23,9 @@ import (
 	"io/ioutil"
 	"sort"
 
+	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
+	"vitess.io/vitess/go/vt/vterrors"
+
 	"vitess.io/vitess/go/json2"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/vt/sqlparser"
@@ -481,7 +484,7 @@ func (vschema *VSchema) findTable(keyspace, tablename string) (*Table, error) {
 	}
 	ks, ok := vschema.Keyspaces[keyspace]
 	if !ok {
-		return nil, fmt.Errorf("keyspace %s not found in vschema", keyspace)
+		return nil, vterrors.NewErrorf(vtrpcpb.Code_NOT_FOUND, vterrors.BadDb, "Unknown database '%s' in vschema", keyspace)
 	}
 	table := ks.Tables[tablename]
 	if table == nil {
@@ -559,7 +562,7 @@ func (vschema *VSchema) FindVindex(keyspace, name string) (Vindex, error) {
 	}
 	ks, ok := vschema.Keyspaces[keyspace]
 	if !ok {
-		return nil, fmt.Errorf("keyspace %s not found in vschema", keyspace)
+		return nil, vterrors.NewErrorf(vtrpcpb.Code_NOT_FOUND, vterrors.BadDb, "Unknown database '%s' in vschema", keyspace)
 	}
 	return ks.Vindexes[name], nil
 }
diff --git a/go/vt/wrangler/stream_migrater.go b/go/vt/wrangler/stream_migrater.go
new file mode 100644
index 0000000000..e01c22fc86
--- /dev/null
+++ b/go/vt/wrangler/stream_migrater.go
@@ -0,0 +1,45 @@
+/*
+Copyright 2019 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 wrangler
+
+import (
+	"context"
+
+	"vitess.io/vitess/go/vt/vtctl/workflow"
+)
+
+type streamMigrater struct {
+	sm *workflow.StreamMigrator
+}
+
+func (sm *streamMigrater) stopStreams(ctx context.Context) ([]string, error) {
+	return sm.sm.StopStreams(ctx)
+}
+
+func (sm *streamMigrater) migrateStreams(ctx context.Context) error {
+	return sm.sm.MigrateStreams(ctx)
+}
+
+func (sm *streamMigrater) cancelMigration(ctx context.Context) {
+	sm.sm.CancelMigration(ctx)
+}
+
+// streamMigraterFinalize finalizes the stream migration.
+// It's a standalone function because it does not use the streamMigrater state.
+func streamMigraterfinalize(ctx context.Context, ts *trafficSwitcher, workflows []string) error {
+	return workflow.StreamMigratorFinalize(ctx, ts, workflows)
+}
diff --git a/go/vt/wrangler/switcher.go b/go/vt/wrangler/switcher.go
index d939b365b9..e5aa998277 100644
--- a/go/vt/wrangler/switcher.go
+++ b/go/vt/wrangler/switcher.go
@@ -81,7 +81,7 @@ func (r *switcher) changeRouting(ctx context.Context) error {
 }
 
 func (r *switcher) streamMigraterfinalize(ctx context.Context, ts *trafficSwitcher, workflows []string) error {
-	return workflow.StreamMigratorFinalize(ctx, ts, workflows)
+	return streamMigraterfinalize(ctx, ts, workflows)
 }
 
 func (r *switcher) createReverseVReplication(ctx context.Context) error {


Test patch:
diff --git a/go/test/endtoend/vtgate/gen4/gen4_test.go b/go/test/endtoend/vtgate/gen4/gen4_test.go
deleted file mode 100644
index aa42b8aa3a..0000000000
--- a/go/test/endtoend/vtgate/gen4/gen4_test.go
+++ /dev/null
@@ -1,71 +0,0 @@
-/*
-Copyright 2021 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"
-	"fmt"
-	"testing"
-
-	"github.com/google/go-cmp/cmp"
-	"github.com/stretchr/testify/require"
-
-	"vitess.io/vitess/go/mysql"
-	"vitess.io/vitess/go/sqltypes"
-)
-
-func TestOrderBy(t *testing.T) {
-	ctx := context.Background()
-	conn, err := mysql.Connect(ctx, &vtParams)
-	require.NoError(t, err)
-	defer conn.Close()
-
-	// insert some data.
-	checkedExec(t, conn, `insert into t1(id, col) values (100, 123),(10, 12),(1, 13),(1000, 1234)`)
-
-	// Gen4 only supported query.
-	assertMatches(t, conn, `select col from t1 order by id`, `[[INT64(13)] [INT64(12)] [INT64(123)] [INT64(1234)]]`)
-
-	// Gen4 unsupported query. v3 supported.
-	assertMatches(t, conn, `select col from t1 order by 1`, `[[INT64(12)] [INT64(13)] [INT64(123)] [INT64(1234)]]`)
-
-	// unsupported in v3 and Gen4.
-	_, err = exec(t, conn, `select t1.* from t1 order by id`)
-	require.Error(t, err)
-}
-
-func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) {
-	t.Helper()
-	qr := checkedExec(t, conn, query)
-	got := fmt.Sprintf("%v", qr.Rows)
-	diff := cmp.Diff(expected, got)
-	if diff != "" {
-		t.Errorf("Query: %s (-want +got):\n%s", query, diff)
-	}
-}
-
-func checkedExec(t *testing.T, conn *mysql.Conn, query string) *sqltypes.Result {
-	t.Helper()
-	qr, err := exec(t, conn, query)
-	require.NoError(t, err, "for query: "+query)
-	return qr
-}
-
-func exec(t *testing.T, conn *mysql.Conn, query string) (*sqltypes.Result, error) {
-	t.Helper()
-	return conn.ExecuteFetch(query, 1000, true)
-}
diff --git a/go/test/endtoend/vtgate/gen4/main_test.go b/go/test/endtoend/vtgate/gen4/main_test.go
deleted file mode 100644
index 03a9bd25a9..0000000000
--- a/go/test/endtoend/vtgate/gen4/main_test.go
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
-Copyright 2021 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 (
-	"flag"
-	"os"
-	"testing"
-
-	"vitess.io/vitess/go/mysql"
-	"vitess.io/vitess/go/test/endtoend/cluster"
-)
-
-var (
-	clusterInstance *cluster.LocalProcessCluster
-	vtParams        mysql.ConnParams
-	KeyspaceName    = "ks"
-	Cell            = "test"
-	SchemaSQL       = `create table t1(
-	id bigint,
-	col bigint,
-	primary key(id)
-) Engine=InnoDB;
-`
-
-	VSchema = `
-{
-  "sharded": true,
-  "vindexes": {
-    "xxhash": {
-      "type": "xxhash"
-    }
-  },
-  "tables": {
-    "t1": {
-      "column_vindexes": [
-        {
-          "column": "id",
-          "name": "xxhash"
-        }
-      ]
-    }
-  }
-}`
-)
-
-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
-		clusterInstance.VtGateExtraArgs = []string{"-planner_version", "Gen4Fallback"} // enable Gen4 planner.
-		err = clusterInstance.StartVtgate()
-		if err != nil {
-			return 1
-		}
-		vtParams = mysql.ConnParams{
-			Host: clusterInstance.Hostname,
-			Port: clusterInstance.VtgateMySQLPort,
-		}
-		return m.Run()
-	}()
-	os.Exit(exitCode)
-}
diff --git a/go/test/endtoend/vtgate/system_schema_test.go b/go/test/endtoend/vtgate/system_schema_test.go
index 8e2f235d34..ad2fb0f5e9 100644
--- a/go/test/endtoend/vtgate/system_schema_test.go
+++ b/go/test/endtoend/vtgate/system_schema_test.go
@@ -78,6 +78,8 @@ func TestInformationSchemaQuery(t *testing.T) {
 	assertResultIsEmpty(t, conn, "table_schema = 'PERFORMANCE_SCHEMA'")
 	assertSingleRowIsReturned(t, conn, "table_schema = 'performance_schema' and table_name = 'users'", "performance_schema")
 	assertResultIsEmpty(t, conn, "table_schema = 'performance_schema' and table_name = 'foo'")
+	assertSingleRowIsReturned(t, conn, "table_schema = 'vt_ks' and table_name = 't1'", "vt_ks")
+	assertSingleRowIsReturned(t, conn, "table_schema = 'ks' and table_name = 't1'", "vt_ks")
 }
 
 func assertResultIsEmpty(t *testing.T, conn *mysql.Conn, pre string) {
diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go
index dd345da3c6..08e0e79440 100644
--- a/go/vt/vtgate/executor_select_test.go
+++ b/go/vt/vtgate/executor_select_test.go
@@ -70,8 +70,8 @@ func TestSelectDBA(t *testing.T) {
 	require.NoError(t, err)
 	wantQueries := []*querypb.BoundQuery{{Sql: query, BindVariables: map[string]*querypb.BindVariable{}}}
 	utils.MustMatch(t, wantQueries, sbc1.Queries)
-	sbc1.Queries = nil
 
+	sbc1.Queries = nil
 	query = "SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES WHERE table_schema = 'performance_schema' AND table_name = 'foo'"
 	_, err = executor.Execute(context.Background(), "TestSelectDBA",
 		NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"}),
@@ -85,6 +85,33 @@ func TestSelectDBA(t *testing.T) {
 		}}}
 	utils.MustMatch(t, wantQueries, sbc1.Queries)
 
+	sbc1.Queries = nil
+	query = "select 1 from information_schema.table_constraints where constraint_schema = 'vt_ks' and table_name = 'user'"
+	_, err = executor.Execute(context.Background(), "TestSelectDBA",
+		NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"}),
+		query, map[string]*querypb.BindVariable{},
+	)
+	require.NoError(t, err)
+	wantQueries = []*querypb.BoundQuery{{Sql: "select 1 from information_schema.table_constraints where constraint_schema = :__vtschemaname and table_name = :__vttablename",
+		BindVariables: map[string]*querypb.BindVariable{
+			"__vtschemaname": sqltypes.StringBindVariable("vt_ks"),
+			"__vttablename":  sqltypes.StringBindVariable("user"),
+		}}}
+	utils.MustMatch(t, wantQueries, sbc1.Queries)
+
+	sbc1.Queries = nil
+	query = "select 1 from information_schema.table_constraints where constraint_schema = 'vt_ks'"
+	_, err = executor.Execute(context.Background(), "TestSelectDBA",
+		NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"}),
+		query, map[string]*querypb.BindVariable{},
+	)
+	require.NoError(t, err)
+	wantQueries = []*querypb.BoundQuery{{Sql: "select 1 from information_schema.table_constraints where constraint_schema = :__vtschemaname",
+		BindVariables: map[string]*querypb.BindVariable{
+			"__vtschemaname": sqltypes.StringBindVariable("vt_ks"),
+		}}}
+	utils.MustMatch(t, wantQueries, sbc1.Queries)
+
 }
 
 func TestUnsharded(t *testing.T) {
diff --git a/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt b/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt
index 091f0bc8dd..b7b14e4ce3 100644
--- a/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/ddl_cases.txt
@@ -94,7 +94,7 @@
 
 #Alter table with unknown qualifier
 "alter table a.b ADD id int"
-"keyspace a not found in vschema"
+"Unknown database 'a' in vschema"
 
 # create db foo
 "create database foo"
diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt
index b8a07e2d0b..b872815cac 100644
--- a/go/vt/vtgate/planbuilder/testdata/from_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/from_cases.txt
@@ -239,7 +239,7 @@ Gen4 plan same as above
 
 # routing rules bad table
 "select * from bad_table"
-"keyspace noks not found in vschema"
+"Unknown database 'noks' in vschema"
 Gen4 plan same as above
 
 # routing rules disabled table
diff --git a/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt b/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt
index 5b25626011..e07ea850cb 100644
--- a/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/memory_sort_cases.txt
@@ -570,24 +570,3 @@
   }
 }
 Gen4 plan same as above
-
-# scatter order by with order by column not present
-"select col from user order by id"
-"unsupported: in scatter query: order by must reference a column in the select list: id asc"
-{
-  "QueryType": "SELECT",
-  "Original": "select col from user order by id",
-  "Instructions": {
-    "OperatorType": "Route",
-    "Variant": "SelectScatter",
-    "Keyspace": {
-      "Name": "user",
-      "Sharded": true
-    },
-    "FieldQuery": "select col, id, weight_string(id) from `user` where 1 != 1",
-    "OrderBy": "1 ASC",
-    "Query": "select col, id, weight_string(id) from `user` order by id asc",
-    "ResultColumns": 1,
-    "Table": "`user`"
-  }
-}
diff --git a/go/vt/vtgate/vindexes/vschema_test.go b/go/vt/vtgate/vindexes/vschema_test.go
index 823b46f4e8..5c06823f09 100644
--- a/go/vt/vtgate/vindexes/vschema_test.go
+++ b/go/vt/vtgate/vindexes/vschema_test.go
@@ -772,7 +772,7 @@ func TestVSchemaRoutingRules(t *testing.T) {
 				Error: errors.New("table t1 must be qualified"),
 			},
 			"badkeyspace": {
-				Error: errors.New("keyspace ks3 not found in vschema"),
+				Error: errors.New("Unknown database 'ks3' in vschema"),
 			},
 			"notfound": {
 				Error: errors.New("table t2 not found"),
@@ -2156,33 +2156,24 @@ func TestFindTable(t *testing.T) {
 	}
 	vschema, _ := BuildVSchema(&input)
 	_, err := vschema.FindTable("", "t1")
-	wantErr := "ambiguous table reference: t1"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr)
-	}
+	require.EqualError(t, err, "ambiguous table reference: t1")
+
 	_, err = vschema.FindTable("", "none")
-	wantErr = "table none not found"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr)
-	}
-	got, err := vschema.FindTable("", "ta")
-	if err != nil {
-		t.Error(err)
-		return
-	}
+	require.EqualError(t, err, "table none not found")
+
 	ta := &Table{
 		Name: sqlparser.NewTableIdent("ta"),
 		Keyspace: &Keyspace{
 			Name: "ksa",
 		},
 	}
-	if !reflect.DeepEqual(got, ta) {
-		t.Errorf("FindTable(\"t1a\"): %+v, want %+v", got, ta)
-	}
+	got, err := vschema.FindTable("", "ta")
+	require.NoError(t, err)
+	require.Equal(t, ta, got)
+
 	got, _ = vschema.FindTable("ksa", "ta")
-	if !reflect.DeepEqual(got, ta) {
-		t.Errorf("FindTable(\"t1a\"): %+v, want %+v", got, ta)
-	}
+	require.Equal(t, ta, got)
+
 	none := &Table{
 		Name: sqlparser.NewTableIdent("none"),
 		Keyspace: &Keyspace{
@@ -2190,19 +2181,13 @@ func TestFindTable(t *testing.T) {
 		},
 	}
 	got, _ = vschema.FindTable("ksa", "none")
-	if !reflect.DeepEqual(got, none) {
-		t.Errorf("FindTable(\"t1a\"): %+v, want %+v", got, none)
-	}
+	require.Equal(t, none, got)
+
 	_, err = vschema.FindTable("ksb", "none")
-	wantErr = "table none not found"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr)
-	}
+	require.EqualError(t, err, "table none not found")
+
 	_, err = vschema.FindTable("none", "aa")
-	wantErr = "keyspace none not found in vschema"
-	if err == nil || err.Error() != wantErr {
-		t.Errorf("FindTable(\"\"): %v, want %s", err, wantErr)
-	}
+	require.EqualError(t, err, "Unknown database 'none' in vschema")
 }
 
 func TestFindTableOrVindex(t *testing.T) {
diff --git a/go/vt/vtgate/vtgate_test.go b/go/vt/vtgate/vtgate_test.go
index 35b0f668cf..28a04110c6 100644
--- a/go/vt/vtgate/vtgate_test.go
+++ b/go/vt/vtgate/vtgate_test.go
@@ -149,7 +149,7 @@ func TestVTGateExecuteWithKeyspaceShard(t *testing.T) {
 		"select id from none",
 		nil,
 	)
-	want := "keyspace invalid_keyspace not found in vschema"
+	want := "Unknown database 'invalid_keyspace' in vschema"
 	assert.EqualError(t, err, want)
 
 	// Valid keyspace/shard.