Merge pull request #13364 from planetscale/cherry-pick-backport-on-re…

…lease-17.0

Cherry-pick all pending PRs into `release-17.0`

Makefile

@@ -460,8 +460,5 @@ generate_ci_workflows:
 generate-flag-testdata:
 	./tools/generate_flag_testdata.sh
 
-release-notes:
-	go run ./go/tools/release-notes --from "$(FROM)" --to "$(TO)" --version "$(VERSION)" --summary "$(SUMMARY)"
-
 install_kubectl_kind:
 	./tools/get_kubectl_kind.sh

changelog/16.0/16.0.0/release_notes.md

@@ -5,6 +5,7 @@
 
 - **[Known Issues](#known-issues)**
   - [MySQL & Xtrabackup known issue](#mysql-xtrabackup-ddl)
+  - [VTTablet Restore Metrics](#vttablet-restore-metrics)
 - **[Major Changes](#major-changes)**
   - **[Breaking Changes](#breaking-changes)**
     - [VTGate Advertised MySQL Version](#advertised-mysql-version)
@@ -100,6 +101,11 @@ or
 > ALTER TABLE your_table ENGINE=InnoDB;
 ```
 
+#### <a id="vttablet-restore-metrics">VTTablet Restore Metrics
+
+As part of the VTTablet Sidecar Schema Maintenance Refactor in v16.0.0, we dropped the `local_metadata` table from the sidecar database schema. This table was storing a couple of metrics related to restores from backup, which have now been lost.
+They have been re-introduced in v17.0.0 as metrics that can be accessed from `/debug/vars`.
+
 ## <a id="major-changes"/>Major Changes
 
 ### <a id="breaking-changes"/>Breaking Changes

changelog/17.0/17.0.0/release_notes.md

@@ -12,6 +12,7 @@
     - [Shard name validation in TopoServer](#shard-name-validation)
     - [Compression CLI flags removed from vtctld and vtctldclient binaries](#remove-compression-flags-from-vtctld-binaries)
     - [VtctldClient command RestoreFromBackup will now use the correct context](#vtctldclient-command-restorefrombackup-will-now-use-the-correct-context)
+    - [VTTablet Restore Metrics](#vttablet-restore-metrics)
   - **[New command line flags and behavior](#new-flag)**
     - [Builtin backup: read buffering flags](#builtin-backup-read-buffering-flags)
     - [Manifest backup external decompressor command](#manifest-backup-external-decompressor-command)
@@ -132,6 +133,11 @@ Prior to v17, this asynchronous process could run indefinitely in the background
 this behavior was changed to use a context with a timeout of `action_timeout`. If you are using VtctldClient to initiate a restore, make sure you provide an appropriate value for action_timeout to give enough
 time for the restore process to complete. Otherwise, the restore will throw an error if the context expires before it completes.
 
+#### <a id="vttablet-restore-metrics">VTTablet Restore Metrics
+
+As part of the VTTablet Sidecar Schema Maintenance Refactor in v16.0.0, we dropped the `local_metadata` table from the sidecar database schema. This table was storing a couple of metrics related to restores from backup.
+They have now been re-introduced as metrics that can be accessed from `/debug/vars`.
+
 ### <a id="Vttablet-TxThrottler">Vttablet's transaction throttler now also throttles DML outside of `BEGIN; ...; COMMIT;` blocks
 
 Prior to v17, `vttablet`'s transaction throttler (enabled with `--enable-tx-throttler`) would only throttle requests done inside an explicit transaction, i.e., a `BEGIN; ...; COMMIT;` block.

doc/internal/ReleaseInstructions.md

@@ -271,12 +271,16 @@ We need to verify that _arewefastyet_ has finished the benchmark too.
     2. Run the following command to generate the release notes:
         1. Release Candidate:
             ```shell
-            make VERSION="v15.0.0-rc1" FROM="v14.0.3" TO="HEAD" SUMMARY="./changelog/15.0/15.0.0/summary.md" release-notes  
+            go run ./go/tools/release-notes --from "v14.0.3" --to "HEAD" --version "v15.0.0-rc1" --summary "./changelog/15.0/15.0.0/summary.md" [--threads=[0-9.]]
             ```
         2. General Availability:
             ```shell
-            make VERSION="v15.0.0-rc1" FROM="v14.0.3" TO="HEAD" SUMMARY="./changelog/15.0/15.0.0/summary.md" release-notes  
+            go run ./go/tools/release-notes --from "v14.0.3" --to "HEAD" --version "v15.0.0" --summary "./changelog/15.0/15.0.0/summary.md" [--threads=[0-9.]]
             ```
+    
+        > Important note: The release note generation fetches a lot of data from the GitHub API. You might reach the API request limit.
+        In which case you should use the `--threads=` flag and set an integer value lower than 10 (the default).
+
        This command will generate the release notes by looking at all the commits between the tag `v14.0.3` and the reference `HEAD`.
        It will also use the file located in `./changelog/15.0/15.0.0/summary.md` to prefix the release notes with a text that the maintainers wrote before the release.
        Please verify the generated release notes to make sure it is well-formatted and all the bookmarks are generated properly.

go/cmd/vtctldclient/command/reparents.go

@@ -80,7 +80,7 @@ EmergencyReparentShard should be used instead.
 		Short: "Updates the topology record for the tablet's shard to acknowledge that an external tool made this tablet the primary.",
 		Long: `Updates the topology record for the tablet's shard to acknowledge that an external tool made this tablet the primary.
 
-See the Reparenting guide for more information: https://vitess.io/docs/user-guides/reparenting/#external-reparenting.
+See the Reparenting guide for more information: https://vitess.io/docs/user-guides/configuration-advanced/reparenting/#external-reparenting.
 `,
 		DisableFlagsInUseLine: true,
 		Args:                  cobra.ExactArgs(1),

go/test/endtoend/backup/vtctlbackup/backup_utils.go

@@ -1233,11 +1233,27 @@ func verifyTabletBackupStats(t *testing.T, vars map[string]any) {
 	if backupstorage.BackupStorageImplementation == "file" {
 		require.Contains(t, bd, "BackupStorage.File.File:Write")
 	}
+
+}
+
+func verifyRestorePositionAndTimeStats(t *testing.T, vars map[string]any) {
+	backupPosition := vars["RestorePosition"].(string)
+	backupTime := vars["RestoredBackupTime"].(string)
+	require.Contains(t, vars, "RestoredBackupTime")
+	require.Contains(t, vars, "RestorePosition")
+	require.NotEqual(t, "", backupPosition)
+	require.NotEqual(t, "", backupTime)
+	rp, err := mysql.DecodePosition(backupPosition)
+	require.NoError(t, err)
+	require.False(t, rp.IsZero())
 }
 
 func verifyTabletRestoreStats(t *testing.T, vars map[string]any) {
 	// Currently only the builtin backup engine instruments bytes-processed
 	// counts.
+
+	verifyRestorePositionAndTimeStats(t, vars)
+
 	if !useXtrabackup {
 		require.Contains(t, vars, "RestoreBytes")
 		bb := vars["RestoreBytes"].(map[string]any)

go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go

@@ -371,6 +371,9 @@ func TestAggregateRandom(t *testing.T) {
 	mcmp.Exec("insert into t2(id, shardKey) values (1, 10), (2, 20)")
 
 	mcmp.AssertMatches("SELECT /*vt+ PLANNER=gen4 */ t1.shardKey, t1.name, count(t2.id) FROM t1 JOIN t2 ON t1.value != t2.shardKey GROUP BY t1.t1_id", `[[INT64(1) VARCHAR("name 1") INT64(2)] [INT64(2) VARCHAR("name 2") INT64(2)]]`)
+
+	mcmp.Exec("set sql_mode=''")
+	mcmp.AssertMatches("select /*vt+ PLANNER=Gen4 */ tbl0.comm, count(*) from emp as tbl0, emp as tbl1 where tbl0.empno = tbl1.deptno", `[[NULL INT64(0)]]`)
 }
 
 // TestAggregateLeftJoin tests that aggregates work with left joins and does not ignore the count when column value does not match the right side table.
@@ -383,8 +386,10 @@ func TestAggregateLeftJoin(t *testing.T) {
 
 	mcmp.AssertMatchesNoOrder("SELECT t1.shardkey FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[INT64(1)] [INT64(0)]]`)
 	mcmp.AssertMatches("SELECT count(t1.shardkey) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[INT64(2)]]`)
+	mcmp.AssertMatches("SELECT count(t2.shardkey) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[INT64(1)]]`)
 	mcmp.AssertMatches("SELECT count(*) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[INT64(2)]]`)
 	mcmp.AssertMatches("SELECT sum(t1.shardkey) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[DECIMAL(1)]]`)
+	mcmp.AssertMatches("SELECT sum(t2.shardkey) FROM t1 LEFT JOIN t2 ON t1.t1_id = t2.id", `[[DECIMAL(1)]]`)
 	mcmp.AssertMatches("SELECT count(*) FROM t2 LEFT JOIN t1 ON t1.t1_id = t2.id WHERE IFNULL(t1.name, 'NOTSET') = 'r'", `[[INT64(1)]]`)
 }
 
@@ -440,6 +445,9 @@ func TestBuggyQueries(t *testing.T) {
 		"[[NULL NULL INT64(3) NULL NULL INT64(3)] "+
 			"[INT32(100) DECIMAL(300) INT64(3) INT32(100) DECIMAL(300) INT64(3)] "+
 			"[INT32(200) DECIMAL(600) INT64(3) INT32(200) DECIMAL(600) INT64(3)]]")
+
+	mcmp.Exec("select /*vt+ PLANNER=gen4 */sum(tbl1.a), min(tbl0.b) from t10 as tbl0, t10 as tbl1 left join t10 as tbl2 on tbl1.a = tbl2.a and tbl1.b = tbl2.k")
+	mcmp.Exec("select /*vt+ PLANNER=gen4 */count(*) from t10 left join t10 as t11 on t10.a = t11.b where t11.a")
 }
 
 func TestMinMaxAcrossJoins(t *testing.T) {

go/test/endtoend/vtgate/queries/aggregation/schema.sql

@@ -75,4 +75,25 @@ CREATE TABLE t10 (
    k BIGINT PRIMARY KEY,
    a INT,
    b INT
-);
+);
+
+CREATE TABLE emp (
+    empno bigint NOT NULL,
+    ename VARCHAR(10),
+    job VARCHAR(9),
+    mgr bigint,
+    hiredate DATE,
+    sal bigint,
+    comm bigint,
+    deptno bigint,
+    PRIMARY KEY (empno)
+) Engine = InnoDB
+  COLLATE = utf8mb4_general_ci;
+
+CREATE TABLE dept (
+    deptno bigint,
+    dname VARCHAR(14),
+    loc VARCHAR(13),
+    PRIMARY KEY (deptno)
+) Engine = InnoDB
+  COLLATE = utf8mb4_general_ci;

go/test/endtoend/vtgate/queries/aggregation/vschema.json

@@ -131,6 +131,22 @@
           "name": "hash"
         }
       ]
+    },
+    "emp": {
+      "column_vindexes": [
+        {
+          "column": "deptno",
+          "name": "hash"
+        }
+      ]
+    },
+    "dept": {
+      "column_vindexes": [
+        {
+          "column": "deptno",
+          "name": "hash"
+        }
+      ]
     }
   }
 }

go/test/endtoend/vtgate/queries/misc/misc_test.go

@@ -303,3 +303,13 @@ func TestPrepareStatements(t *testing.T) {
 	_, err = mcmp.ExecAllowAndCompareError("deallocate prepare prep_art")
 	assert.ErrorContains(t, err, "VT09011: Unknown prepared statement handler (prep_art) given to DEALLOCATE PREPARE")
 }
+
+func TestBuggyOuterJoin(t *testing.T) {
+	// We found a couple of inconsistencies around outer joins, adding these tests to stop regressions
+	mcmp, closer := start(t)
+	defer closer()
+
+	mcmp.Exec("insert into t1(id1, id2) values (1,2), (42,5), (5, 42)")
+
+	mcmp.Exec("select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2")
+}

go/tools/release-notes/release_notes.go

@@ -82,6 +82,7 @@ type (
 
 var (
 	releaseNotesPath = `changelog/`
+	numberOfThreads  = 10
 )
 
 const (
@@ -134,7 +135,6 @@ The entire changelog for this release can be found [here]({{ .PathToChangeLogFil
 
 	prefixType        = "Type: "
 	prefixComponent   = "Component: "
-	numberOfThreads   = 10
 	lengthOfSingleSHA = 40
 )
 
@@ -499,8 +499,11 @@ func main() {
 	pflag.StringVarP(&to, "to", "t", to, "to sha/tag/branch")
 	pflag.StringVarP(&versionName, "version", "v", "", "name of the version (has to be the following format: v11.0.0)")
 	pflag.StringVarP(&summaryFile, "summary", "s", "", "readme file on which there is a summary of the release")
+	pflag.IntVar(&numberOfThreads, "threads", numberOfThreads, "Define the number of threads used to fetch data from GitHub's API. Lower this number if you hit request limit errors.")
 	pflag.Parse()
 
+	log.Println(numberOfThreads)
+	os.Exit(1)
 	// The -version flag must be of a valid format.
 	rx := regexp.MustCompile(`v([0-9]+)\.([0-9]+)\.([0-9]+)`)
 	// There should be 4 sub-matches, input: "v14.0.0", output: ["v14.0.0", "14", "0", "0"].

go/vt/mysqlctl/schema.go

@@ -52,9 +52,9 @@ func (mysqld *Mysqld) executeSchemaCommands(sql string) error {
 	return mysqld.executeMysqlScript(params, strings.NewReader(sql))
 }
 
-func encodeTableName(tableName string) string {
+func encodeEntityName(name string) string {
 	var buf strings.Builder
-	sqltypes.NewVarChar(tableName).EncodeSQL(&buf)
+	sqltypes.NewVarChar(name).EncodeSQL(&buf)
 	return buf.String()
 }
 
@@ -66,7 +66,7 @@ func tableListSQL(tables []string) (string, error) {
 
 	encodedTables := make([]string, len(tables))
 	for i, tableName := range tables {
-		encodedTables[i] = encodeTableName(tableName)
+		encodedTables[i] = encodeEntityName(tableName)
 	}
 
 	return "(" + strings.Join(encodedTables, ", ") + ")", nil
@@ -280,7 +280,7 @@ func ResolveTables(ctx context.Context, mysqld MysqlDaemon, dbName string, table
 const (
 	GetColumnNamesQuery = `SELECT COLUMN_NAME as column_name
 		FROM INFORMATION_SCHEMA.COLUMNS
-		WHERE TABLE_SCHEMA = %s AND TABLE_NAME = '%s'
+		WHERE TABLE_SCHEMA = %s AND TABLE_NAME = %s
 		ORDER BY ORDINAL_POSITION`
 	GetFieldsQuery = "SELECT %s FROM %s WHERE 1 != 1"
 )
@@ -290,9 +290,9 @@ func GetColumnsList(dbName, tableName string, exec func(string, int, bool) (*sql
 	if dbName == "" {
 		dbName2 = "database()"
 	} else {
-		dbName2 = fmt.Sprintf("'%s'", dbName)
+		dbName2 = encodeEntityName(dbName)
 	}
-	query := fmt.Sprintf(GetColumnNamesQuery, dbName2, sqlescape.UnescapeID(tableName))
+	query := fmt.Sprintf(GetColumnNamesQuery, dbName2, encodeEntityName(sqlescape.UnescapeID(tableName)))
 	qr, err := exec(query, -1, true)
 	if err != nil {
 		return "", err
@@ -377,9 +377,9 @@ func (mysqld *Mysqld) getPrimaryKeyColumns(ctx context.Context, dbName string, t
 	sql := `
             SELECT TABLE_NAME as table_name, COLUMN_NAME as column_name
             FROM information_schema.STATISTICS
-            WHERE TABLE_SCHEMA = '%s' AND TABLE_NAME IN %s AND LOWER(INDEX_NAME) = 'primary'
+            WHERE TABLE_SCHEMA = %s AND TABLE_NAME IN %s AND LOWER(INDEX_NAME) = 'primary'
             ORDER BY table_name, SEQ_IN_INDEX`
-	sql = fmt.Sprintf(sql, dbName, tableList)
+	sql = fmt.Sprintf(sql, encodeEntityName(dbName), tableList)
 	qr, err := conn.ExecuteFetch(sql, len(tables)*100, true)
 	if err != nil {
 		return nil, err
@@ -599,16 +599,18 @@ func (mysqld *Mysqld) GetPrimaryKeyEquivalentColumns(ctx context.Context, dbName
                                               END
                                             ) AS type_cost, COUNT(stats.COLUMN_NAME) AS col_count FROM information_schema.STATISTICS AS stats INNER JOIN
                   information_schema.COLUMNS AS cols ON stats.TABLE_SCHEMA = cols.TABLE_SCHEMA AND stats.TABLE_NAME = cols.TABLE_NAME AND stats.COLUMN_NAME = cols.COLUMN_NAME
-                WHERE stats.TABLE_SCHEMA = '%s' AND stats.TABLE_NAME = '%s' AND stats.INDEX_NAME NOT IN
+                WHERE stats.TABLE_SCHEMA = %s AND stats.TABLE_NAME = %s AND stats.INDEX_NAME NOT IN
                 (
                     SELECT DISTINCT INDEX_NAME FROM information_schema.STATISTICS
-                    WHERE TABLE_SCHEMA = '%s' AND TABLE_NAME = '%s' AND (NON_UNIQUE = 1 OR NULLABLE = 'YES')
+                    WHERE TABLE_SCHEMA = %s AND TABLE_NAME = %s AND (NON_UNIQUE = 1 OR NULLABLE = 'YES')
                 )
                 GROUP BY INDEX_NAME ORDER BY type_cost ASC, col_count ASC LIMIT 1
             ) AS pke ON index_cols.INDEX_NAME = pke.INDEX_NAME
-            WHERE index_cols.TABLE_SCHEMA = '%s' AND index_cols.TABLE_NAME = '%s' AND NON_UNIQUE = 0 AND NULLABLE != 'YES'
+            WHERE index_cols.TABLE_SCHEMA = %s AND index_cols.TABLE_NAME = %s AND NON_UNIQUE = 0 AND NULLABLE != 'YES'
             ORDER BY SEQ_IN_INDEX ASC`
-	sql = fmt.Sprintf(sql, dbName, table, dbName, table, dbName, table)
+	encodedDbName := encodeEntityName(dbName)
+	encodedTable := encodeEntityName(table)
+	sql = fmt.Sprintf(sql, encodedDbName, encodedTable, encodedDbName, encodedTable, encodedDbName, encodedTable)
 	qr, err := conn.ExecuteFetch(sql, 1000, true)
 	if err != nil {
 		return nil, err

go/vt/mysqlctl/schema_test.go

@@ -15,7 +15,7 @@ var queryMap map[string]*sqltypes.Result
 
 func mockExec(query string, maxRows int, wantFields bool) (*sqltypes.Result, error) {
 	queryMap = make(map[string]*sqltypes.Result)
-	getColsQuery := fmt.Sprintf(GetColumnNamesQuery, "'test'", "t1")
+	getColsQuery := fmt.Sprintf(GetColumnNamesQuery, "'test'", "'t1'")
 	queryMap[getColsQuery] = &sqltypes.Result{
 		Fields: []*querypb.Field{{
 			Name: "column_name",
@@ -40,7 +40,7 @@ func mockExec(query string, maxRows int, wantFields bool) (*sqltypes.Result, err
 			Type: sqltypes.VarBinary,
 		}},
 	}
-	getColsQuery = fmt.Sprintf(GetColumnNamesQuery, "database()", "t2")
+	getColsQuery = fmt.Sprintf(GetColumnNamesQuery, "database()", "'t2'")
 	queryMap[getColsQuery] = &sqltypes.Result{
 		Fields: []*querypb.Field{{
 			Name: "column_name",
@@ -61,6 +61,29 @@ func mockExec(query string, maxRows int, wantFields bool) (*sqltypes.Result, err
 	if ok {
 		return result, nil
 	}
+
+	getColsQuery = fmt.Sprintf(GetColumnNamesQuery, "database()", "'with \\' quote'")
+	queryMap[getColsQuery] = &sqltypes.Result{
+		Fields: []*querypb.Field{{
+			Name: "column_name",
+			Type: sqltypes.VarChar,
+		}},
+		Rows: [][]sqltypes.Value{
+			{sqltypes.NewVarChar("col1")},
+		},
+	}
+
+	queryMap["SELECT `col1` FROM `with ' quote` WHERE 1 != 1"] = &sqltypes.Result{
+		Fields: []*querypb.Field{{
+			Name: "col1",
+			Type: sqltypes.VarChar,
+		}},
+	}
+	result, ok = queryMap[query]
+	if ok {
+		return result, nil
+	}
+
 	return nil, fmt.Errorf("query %s not found in mock setup", query)
 }
 
@@ -74,4 +97,9 @@ func TestColumnList(t *testing.T) {
 	fields, _, err = GetColumns("", "t2", mockExec)
 	require.NoError(t, err)
 	require.Equal(t, `[name:"col1" type:VARCHAR]`, fmt.Sprintf("%+v", fields))
+
+	fields, _, err = GetColumns("", "with ' quote", mockExec)
+	require.NoError(t, err)
+	require.Equal(t, `[name:"col1" type:VARCHAR]`, fmt.Sprintf("%+v", fields))
+
 }

go/vt/vtgate/engine/filter.go

@@ -67,11 +67,8 @@ func (f *Filter) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[s
 		if err != nil {
 			return nil, err
 		}
-		intEvalResult, err := evalResult.Value().ToInt64()
-		if err != nil {
-			return nil, err
-		}
-		if intEvalResult == 1 {
+
+		if evalResult.ToBoolean() {
 			rows = append(rows, row)
 		}
 	}

go/vt/vtgate/engine/projection.go

@@ -129,7 +129,7 @@ func (p *Projection) GetFields(ctx context.Context, vcursor VCursor, bindVars ma
 		return nil, err
 	}
 	env := evalengine.NewExpressionEnv(ctx, bindVars, vcursor)
-	qr.Fields, err = p.evalFields(env, nil)
+	qr.Fields, err = p.evalFields(env, qr.Fields)
 	if err != nil {
 		return nil, err
 	}

go/vt/vtgate/engine/scalar_aggregation.go

@@ -209,7 +209,8 @@ func createEmptyValueFor(opcode AggregateOpcode) (sqltypes.Value, error) {
 		AggregateSumDistinct,
 		AggregateSum,
 		AggregateMin,
-		AggregateMax:
+		AggregateMax,
+		AggregateRandom:
 		return sqltypes.NULL, nil
 
 	}