enable schema tracking by default (#10455)

* feat: enable schema tracking by default Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: fix test setup Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: fix vschema test setup Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: turn off schema tracking on the tablet Signed-off-by: Andres Taylor <andres@planetscale.com> * test: fix test assertion Signed-off-by: Andres Taylor <andres@planetscale.com> * Change read query for checks that test to which keyspace a table is routed to. This uses the /queryz vttablet endpoint where the query gets expanded if schema tracking is enabled, hence failing an exact query check Signed-off-by: Rohit Nayak <rohit@planetscale.com> * test: only use gen4 planner Signed-off-by: Harshit Gangal <harshit@planetscale.com> * fix: column list population in insert with auto-inc column Signed-off-by: Harshit Gangal <harshit@planetscale.com> * fix: change parser to keep empty column list as provided by user Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: fix test expectation Signed-off-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Andres Taylor <andres@planetscale.com> Co-authored-by: Rohit Nayak <rohit@planetscale.com> Signed-off-by: Harshit Gangal <harshit@planetscale.com>
vitessio · Jun 28, 2022 · 3d5f18a · 3d5f18a
1 parent 1fee520
commit 3d5f18a
Show file tree

Hide file tree

Showing 23 changed files with 1,480 additions and 32 deletions.
diff --git a/go/flags/endtoend/flags_test.go b/go/flags/endtoend/flags_test.go
@@ -1460,11 +1460,13 @@ max_rate_approach_threshold: 0.9
 func TestHelpOutput(t *testing.T) {
 	args := []string{"--help"}
 	for binary, helptext := range helpOutput {
-		cmd := exec.Command(binary, args...)
-		output := bytes.Buffer{}
-		cmd.Stderr = &output
-		err := cmd.Run()
-		require.NoError(t, err)
-		assert.Equal(t, helptext, output.String(), fmt.Sprintf("%s does not have the expected help output. Please update the test if you intended to change it.", binary))
+		t.Run(binary, func(t *testing.T) {
+			cmd := exec.Command(binary, args...)
+			output := bytes.Buffer{}
+			cmd.Stderr = &output
+			err := cmd.Run()
+			require.NoError(t, err)
+			assert.Equal(t, helptext, output.String(), fmt.Sprintf("%s does not have the expected help output. Please update the test if you intended to change it.", binary))
+		})
 	}
 }
diff --git a/go/flags/endtoend/vtgate.txt b/go/flags/endtoend/vtgate.txt
diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt
diff --git a/go/test/endtoend/vreplication/resharding_workflows_v2_test.go b/go/test/endtoend/vreplication/resharding_workflows_v2_test.go
@@ -39,7 +39,7 @@ const (
 	reverseKsWorkflow = sourceKs + "." + workflowName + "_reverse"
 	tablesToMove      = "customer"
 	defaultCellName   = "zone1"
-	readQuery         = "select * from customer"
+	readQuery         = "select cid from customer"
 )
 
 const (

diff --git a/go/test/endtoend/vtgate/errors_as_warnings/main_test.go b/go/test/endtoend/vtgate/errors_as_warnings/main_test.go
@@ -145,10 +145,10 @@ func TestScatterErrsAsWarns(t *testing.T) {
 			assertContainsOneOf(t, mode.conn, showQ, "no valid tablet", "no healthy tablet", "mysql.sock: connect: no such file or directory")
 
 			// invalid_field should throw error and not warning
-			_, err = mode.conn.ExecuteFetch("SELECT /*vt+ SCATTER_ERRORS_AS_WARNINGS */ invalid_field from t1;", 1, false)
+			_, err = mode.conn.ExecuteFetch("SELECT /*vt+ PLANNER=Gen4 SCATTER_ERRORS_AS_WARNINGS */ invalid_field from t1;", 1, false)
 			require.Error(t, err)
 			serr := mysql.NewSQLErrorFromError(err).(*mysql.SQLError)
-			require.Equal(t, 1054, serr.Number())
+			require.Equal(t, 1054, serr.Number(), serr.Error())
 		})
 	}
 }

diff --git a/go/test/endtoend/vtgate/schematracker/loadkeyspace/schema_load_keyspace_test.go b/go/test/endtoend/vtgate/schematracker/loadkeyspace/schema_load_keyspace_test.go
@@ -69,6 +69,7 @@ func TestBlockedLoadKeyspace(t *testing.T) {
 		Name:      keyspaceName,
 		SchemaSQL: sqlSchema,
 	}
+	clusterInstance.VtTabletExtraArgs = []string{"--queryserver-config-schema-change-signal=false"}
 	err = clusterInstance.StartUnshardedKeyspace(*keyspace, 0, false)
 	require.NoError(t, err)
 

diff --git a/go/test/endtoend/vtgate/vschema/vschema_test.go b/go/test/endtoend/vtgate/vschema/vschema_test.go
@@ -66,7 +66,7 @@ func TestMain(m *testing.M) {
 		}
 
 		// List of users authorized to execute vschema ddl operations
-		clusterInstance.VtGateExtraArgs = []string{"--vschema_ddl_authorized_users=%"}
+		clusterInstance.VtGateExtraArgs = []string{"--vschema_ddl_authorized_users=%", "--schema_change_signal=false"}
 
 		// Start keyspace
 		keyspace := &cluster.Keyspace{

diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go
@@ -1082,7 +1082,8 @@ func (node Columns) Format(buf *TrackedBuffer) {
 	if node == nil {
 		return
 	}
-	prefix := "("
+	buf.WriteByte('(')
+	prefix := ""
 	for _, n := range node {
 		buf.astPrintf(node, "%s%v", prefix, n)
 		prefix = ", "

diff --git a/go/vt/sqlparser/ast_format_fast.go b/go/vt/sqlparser/ast_format_fast.go
diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go
@@ -946,8 +946,7 @@ var (
 		input:  "insert into user(format, tree, vitess) values ('Chuck', 42, 'Barry')",
 		output: "insert into `user`(`format`, `tree`, `vitess`) values ('Chuck', 42, 'Barry')",
 	}, {
-		input:  "insert into customer () values ()",
-		output: "insert into customer values ()",
+		input: "insert into customer() values ()",
 	}, {
 		input: "update /* simple */ a set b = 3",
 	}, {

diff --git a/go/vt/sqlparser/sql.go b/go/vt/sqlparser/sql.go
diff --git a/go/vt/sqlparser/sql.y b/go/vt/sqlparser/sql.y
@@ -6765,7 +6765,7 @@ insert_data:
   }
 | openb closeb VALUES tuple_list
   {
-    $$ = &Insert{Rows: $4}
+    $$ = &Insert{Columns: []ColIdent{}, Rows: $4}
   }
 | openb ins_column_list closeb select_statement
   {

diff --git a/go/vt/vtgate/engine/cached_size.go b/go/vt/vtgate/engine/cached_size.go
diff --git a/go/vt/vtgate/engine/lock.go b/go/vt/vtgate/engine/lock.go
@@ -186,7 +186,7 @@ func (l *Lock) description() PrimitiveDescription {
 	}
 	var lf []string
 	for _, f := range l.LockFunctions {
-		lf = append(lf, fmt.Sprintf("%s", sqlparser.String(f.Typ)))
+		lf = append(lf, sqlparser.String(f.Typ))
 	}
 	other["lock_func"] = lf
 	return PrimitiveDescription{

diff --git a/go/vt/vtgate/executor_dml_test.go b/go/vt/vtgate/executor_dml_test.go
@@ -1993,7 +1993,7 @@ func TestReservedConnDML(t *testing.T) {
 
 	wantQueries = append(wantQueries,
 		&querypb.BoundQuery{Sql: "set @@default_week_format = 1", BindVariables: map[string]*querypb.BindVariable{}},
-		&querypb.BoundQuery{Sql: "insert into `simple` values ()", BindVariables: map[string]*querypb.BindVariable{}})
+		&querypb.BoundQuery{Sql: "insert into `simple`() values ()", BindVariables: map[string]*querypb.BindVariable{}})
 	_, err = executor.Execute(ctx, "TestReservedConnDML", session, "insert into `simple`() values ()", nil)
 	require.NoError(t, err)
 	assertQueries(t, sbc, wantQueries)
@@ -2008,7 +2008,7 @@ func TestReservedConnDML(t *testing.T) {
 	// as the first time the query fails due to connection loss i.e. reserved conn lost. It will be recreated to set statement will be executed again.
 	wantQueries = append(wantQueries,
 		&querypb.BoundQuery{Sql: "set @@default_week_format = 1", BindVariables: map[string]*querypb.BindVariable{}},
-		&querypb.BoundQuery{Sql: "insert into `simple` values ()", BindVariables: map[string]*querypb.BindVariable{}})
+		&querypb.BoundQuery{Sql: "insert into `simple`() values ()", BindVariables: map[string]*querypb.BindVariable{}})
 	_, err = executor.Execute(ctx, "TestReservedConnDML", session, "insert into `simple`() values ()", nil)
 	require.NoError(t, err)
 	assertQueries(t, sbc, wantQueries)
@@ -2050,7 +2050,7 @@ func TestStreamingDML(t *testing.T) {
 		openTx:      true,
 		changedRows: 1,
 		expQuery: []*querypb.BoundQuery{{
-			Sql:           "insert into `simple` values ()",
+			Sql:           "insert into `simple`() values ()",
 			BindVariables: map[string]*querypb.BindVariable{},
 		}},
 	}, {

diff --git a/go/vt/vtgate/executor_framework_test.go b/go/vt/vtgate/executor_framework_test.go
@@ -664,9 +664,7 @@ var testPlannedQueries = map[string]bool{}
 func testQueryLog(t *testing.T, logChan chan any, method, stmtType, sql string, shardQueries int) *LogStats {
 	t.Helper()
 
-	var logStats *LogStats
-
-	logStats = getQueryLog(logChan)
+	logStats := getQueryLog(logChan)
 	require.NotNil(t, logStats)
 
 	var log bytes.Buffer

diff --git a/go/vt/vtgate/planbuilder/insert.go b/go/vt/vtgate/planbuilder/insert.go
@@ -98,7 +98,9 @@ func buildInsertUnshardedPlan(ins *sqlparser.Insert, table *vindexes.Table, rese
 		eins.Query = generateQuery(ins)
 	} else {
 		// Table has auto-inc and has a VALUES clause.
-		if len(ins.Columns) == 0 {
+		// If the column list is nil then add all the columns
+		// If the column list is empty then add only the auto-inc column and this happens on calling modifyForAutoinc
+		if ins.Columns == nil {
 			if table.ColumnListAuthoritative {
 				populateInsertColumnlist(ins, table)
 			} else {
@@ -131,10 +133,8 @@ func buildInsertShardedPlan(ins *sqlparser.Insert, table *vindexes.Table, reserv
 		}
 		eins.Ignore = true
 	}
-	if len(ins.Columns) == 0 {
-		if table.ColumnListAuthoritative {
-			populateInsertColumnlist(ins, table)
-		}
+	if ins.Columns == nil && table.ColumnListAuthoritative {
+		populateInsertColumnlist(ins, table)
 	}
 
 	applyCommentDirectives(ins, eins)

diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.txt b/go/vt/vtgate/planbuilder/testdata/dml_cases.txt
@@ -4472,3 +4472,26 @@ Gen4 plan same as above
     "Table": "user"
   }
 }
+
+# all defaults empty column, empty values
+"insert into authoritative () values ()"
+{
+  "QueryType": "INSERT",
+  "Original": "insert into authoritative () values ()",
+  "Instructions": {
+    "OperatorType": "Insert",
+    "Variant": "Sharded",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "TargetTabletType": "PRIMARY",
+    "MultiShardAutocommit": false,
+    "Query": "insert into authoritative(user_id) values (:_user_id_0)",
+    "TableName": "authoritative",
+    "VindexValues": {
+      "user_index": "NULL"
+    }
+  }
+}
+Gen4 plan same as above
diff --git a/go/vt/vtgate/vtgate.go b/go/vt/vtgate/vtgate.go
@@ -95,7 +95,7 @@ var (
 	enableOnlineDDL = flag.Bool("enable_online_ddl", true, "Allow users to submit, review and control Online DDL")
 	enableDirectDDL = flag.Bool("enable_direct_ddl", true, "Allow users to submit direct DDL statements")
 
-	enableSchemaChangeSignal = flag.Bool("schema_change_signal", false, "Enable the schema tracker; requires queryserver-config-schema-change-signal to be enabled on the underlying vttablets for this to work")
+	enableSchemaChangeSignal = flag.Bool("schema_change_signal", true, "Enable the schema tracker; requires queryserver-config-schema-change-signal to be enabled on the underlying vttablets for this to work")
 	schemaChangeUser         = flag.String("schema_change_signal_user", "", "User to be used to send down query to vttablet to retrieve schema changes")
 )
 

diff --git a/go/vt/vtgate/vtgate_test.go b/go/vt/vtgate/vtgate_test.go
@@ -605,7 +605,7 @@ func TestMultiInternalSavepointVtGate(t *testing.T) {
 	sbc3.Queries = nil
 
 	// single shard so no savepoint will be created and neither any old savepoint will be executed
-	session, _, err = rpcVTGate.Execute(context.Background(), session, "insert into sp_tbl(user_id) values (5)", nil)
+	_, _, err = rpcVTGate.Execute(context.Background(), session, "insert into sp_tbl(user_id) values (5)", nil)
 	require.NoError(t, err)
 	wantQ = []*querypb.BoundQuery{{
 		Sql: "insert into sp_tbl(user_id) values (:_user_id_0)",

diff --git a/go/vt/vttablet/tabletserver/health_streamer_test.go b/go/vt/vttablet/tabletserver/health_streamer_test.go
@@ -68,6 +68,7 @@ func TestHealthStreamerBroadcast(t *testing.T) {
 	}
 	blpFunc = testBlpFunc
 	hs := newHealthStreamer(env, alias)
+	hs.InitDBConfig(&querypb.Target{TabletType: topodatapb.TabletType_PRIMARY}, config.DB.DbaWithDB())
 	hs.Open()
 	defer hs.Close()
 	target := &querypb.Target{}

diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go
@@ -494,7 +494,7 @@ var defaultConfig = TabletConfig{
 	SignalSchemaChangeReloadIntervalSeconds: 5,
 	MessagePostponeParallelism:              4,
 	CacheResultFields:                       true,
-	SignalWhenSchemaChange:                  false, // while this feature is experimental, the safe default is off
+	SignalWhenSchemaChange:                  true,
 
 	EnableTxThrottler:           false,
 	TxThrottlerConfig:           defaultTxThrottlerConfig(),

diff --git a/go/vt/vttablet/tabletserver/tabletenv/config_test.go b/go/vt/vttablet/tabletserver/tabletenv/config_test.go
@@ -153,6 +153,7 @@ rowStreamer:
   maxMySQLReplLagSecs: 43200
 schemaReloadIntervalSeconds: 1800
 signalSchemaChangeReloadIntervalSeconds: 5
+signalWhenSchemaChange: true
 streamBufferSize: 32768
 txPool:
   idleTimeoutSeconds: 1800
@@ -217,6 +218,7 @@ func TestFlags(t *testing.T) {
 		QueryCacheLFU:                           cache.DefaultConfig.LFU,
 		SchemaReloadIntervalSeconds:             1800,
 		SignalSchemaChangeReloadIntervalSeconds: 5,
+		SignalWhenSchemaChange:                  true,
 		TrackSchemaVersions:                     false,
 		MessagePostponeParallelism:              4,
 		CacheResultFields:                       true,