[Backport 14.0] enable schema tracking by default (#10595)

* 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>

* test: fix test expectation

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

* fix: Allow gen4 to handle variables on authoritative tables

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

* Empty-Commit

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

* test: system variable selection on a table only works in gen4

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

Co-authored-by: Andres Taylor <andres@planetscale.com>
Co-authored-by: Rohit Nayak <rohit@planetscale.com>

go/flags/endtoend/flags_test.go

@@ -304,7 +304,7 @@ var (
   --retry-count int
 	retry count (default 2)
   --schema_change_signal
-	Enable the schema tracker; requires queryserver-config-schema-change-signal to be enabled on the underlying vttablets for this to work
+	Enable the schema tracker; requires queryserver-config-schema-change-signal to be enabled on the underlying vttablets for this to work (default true)
   --schema_change_signal_user string
 	User to be used to send down query to vttablet to retrieve schema changes
   --security_policy string
@@ -1110,7 +1110,7 @@ var (
   --queryserver-config-query-timeout float
 	query server query timeout (in seconds), this is the query timeout in vttablet side. If a query takes more than this timeout, it will be killed. (default 30)
   --queryserver-config-schema-change-signal
-	query server schema signal, will signal connected vtgates that schema has changed whenever this is detected. VTGates will need to have -schema_change_signal enabled for this to work
+	query server schema signal, will signal connected vtgates that schema has changed whenever this is detected. VTGates will need to have -schema_change_signal enabled for this to work (default true)
   --queryserver-config-schema-change-signal-interval float
 	query server schema change signal interval defines at which interval the query server shall send schema updates to vtgate. (default 5)
   --queryserver-config-schema-reload-time float
@@ -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))
+		})
 	}
 }

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 (

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())
 		})
 	}
 }

go/test/endtoend/vtgate/reservedconn/sysvar_test.go

@@ -128,7 +128,7 @@ func TestSetSystemVarWithTxFailure(t *testing.T) {
 	utils.Exec(t, conn, "insert into test (id, val1) values (80, null)")
 
 	// before changing any settings, let's confirm sql_safe_updates value
-	utils.AssertMatches(t, conn, `select @@sql_safe_updates from test where id = 80`, `[[INT64(0)]]`)
+	utils.AssertMatches(t, conn, `select /*vt+ PLANNER=gen4 */ @@sql_safe_updates from test where id = 80`, `[[INT64(0)]]`)
 
 	utils.Exec(t, conn, "set sql_safe_updates = 1")
 	utils.Exec(t, conn, "begin")

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)
 

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{

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 = ", "

go/vt/sqlparser/ast_funcs.go

@@ -2012,3 +2012,7 @@ func convertStringToInt(integer string) int {
 	val, _ := strconv.Atoi(integer)
 	return val
 }
+
+func (node *ColName) IsVariable() bool {
+	return node.Name.AtCount() != NoAt
+}

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",
 	}, {

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
   {

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{

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{},
 		}},
 	}, {

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

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)

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

go/vt/vtgate/planbuilder/testdata/select_cases.txt

@@ -3472,3 +3472,23 @@ Gen4 plan same as above
     ]
   }
 }
+
+# Getting variable value from an authoritative table should work just fine
+# V3 still fails. Fixed in v15
+"select @@foo from authoritative"
+"symbol @@foo not found in table or subquery"
+{
+  "QueryType": "SELECT",
+  "Original": "select @@foo from authoritative",
+  "Instructions": {
+    "OperatorType": "Route",
+    "Variant": "Scatter",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "FieldQuery": "select @@foo from authoritative where 1 != 1",
+    "Query": "select @@foo from authoritative",
+    "Table": "authoritative"
+  }
+}

go/vt/vtgate/semantics/binder.go

@@ -91,6 +91,9 @@ func (b *binder) up(cursor *sqlparser.Cursor) error {
 			node.Using = nil
 		}
 	case *sqlparser.ColName:
+		if node.IsVariable() {
+			break
+		}
 		currentScope := b.scoper.currentScope()
 		deps, err := b.resolveColumn(node, currentScope, false)
 		if err != nil {

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")
 )
 

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)",

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{}

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(),

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,