Allow zero (in) date when setting up internal _vt schema (#12262)

* Allow zero (in) date when setting up internal _vt schema Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * modify test sto include @@sql_mod query support Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Allow zero (in) date when setting up internal _vt schema Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * modify test sto include @@sql_mod query support Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Fix test failures Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Fix failing tests. Minor refactor Signed-off-by: Rohit Nayak <rohit@planetscale.com> * change sql_mode t omost permissive (empty) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * setPermissiveSQLMode Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * fixes per review Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Add missing mock query Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Signed-off-by: Rohit Nayak <rohit@planetscale.com> Co-authored-by: Rohit Nayak <rohit@planetscale.com>
vitessio · Feb 13, 2023 · 32d77db · 32d77db
1 parent c4292b7
commit 32d77db
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 5 deletions.
diff --git a/go/vt/sidecardb/sidecardb.go b/go/vt/sidecardb/sidecardb.go
@@ -194,6 +194,12 @@ func Init(ctx context.Context, exec Exec) error {
 		return err
 	}
 
+	resetSQLMode, err := si.setPermissiveSQLMode()
+	if err != nil {
+		return err
+	}
+	defer resetSQLMode()
+
 	for _, table := range sidecarTables {
 		if err := si.ensureSchema(table); err != nil {
 			return err
@@ -202,6 +208,31 @@ func Init(ctx context.Context, exec Exec) error {
 	return nil
 }
 
+// setPermissiveSQLMode gets the current sql_mode for the session, removes any
+// restrictions, and returns a function to restore it back to the original session value.
+// We need to allow for the recreation of any data that currently exists in the table, such
+// as e.g. allowing any zero dates that may already exist in a preexisting sidecar table.
+func (si *schemaInit) setPermissiveSQLMode() (func(), error) {
+	rs, err := si.exec(si.ctx, `select @@session.sql_mode as sql_mode`, 1, false)
+	if err != nil {
+		return nil, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "could not read sql_mode: %v", err)
+	}
+	sqlMode, err := rs.Named().Row().ToString("sql_mode")
+	if err != nil {
+		return nil, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "could not read sql_mode: %v", err)
+	}
+
+	resetSQLModeFunc := func() {
+		restoreSQLModeQuery := fmt.Sprintf("set @@session.sql_mode='%s'", sqlMode)
+		_, _ = si.exec(si.ctx, restoreSQLModeQuery, 0, false)
+	}
+
+	if _, err := si.exec(si.ctx, "set @@session.sql_mode=''", 0, false); err != nil {
+		return nil, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "could not change sql_mode: %v", err)
+	}
+	return resetSQLModeFunc, nil
+}
+
 func (si *schemaInit) doesSidecarDBExist() (bool, error) {
 	rs, err := si.exec(si.ctx, ShowSidecarDatabasesQuery, 2, false)
 	if err != nil {
@@ -374,6 +405,15 @@ func AddSchemaInitQueries(db *fakesqldb.DB, populateTables bool) {
 		}
 		db.AddQuery(fmt.Sprintf(ShowCreateTableQuery, table.name), result)
 	}
+
+	sqlModeResult := sqltypes.MakeTestResult(sqltypes.MakeTestFields(
+		"sql_mode",
+		"varchar"),
+		"ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION",
+	)
+	db.AddQuery("select @@session.sql_mode as sql_mode", sqlModeResult)
+
+	db.AddQuery("set @@session.sql_mode=''", &sqltypes.Result{})
 }
 
 // MatchesInitQuery returns true if query has one of the test patterns as a substring, or it matches a provided regexp.

diff --git a/go/vt/sidecardb/sidecardb_test.go b/go/vt/sidecardb/sidecardb_test.go
@@ -27,11 +27,18 @@ import (
 )
 
 // Tests all non-error code paths in sidecardb
-func TestAll(t *testing.T) {
+func TestAllSidecarDB(t *testing.T) {
 	db := fakesqldb.New(t)
 	defer db.Close()
 	AddSchemaInitQueries(db, false)
 	db.AddQuery("use dbname", &sqltypes.Result{})
+	sqlMode := sqltypes.MakeTestResult(sqltypes.MakeTestFields(
+		"sql_mode",
+		"varchar"),
+		"ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION",
+	)
+	db.AddQuery("select @@session.sql_mode as sql_mode", sqlMode)
+	db.AddQueryPattern("set @@session.sql_mode=.*", &sqltypes.Result{})
 
 	ctx := context.Background()
 	cp := db.ConnParams()

diff --git a/go/vt/vtexplain/vtexplain_vttablet.go b/go/vt/vtexplain/vtexplain_vttablet.go
@@ -144,7 +144,7 @@ func (vte *VTExplain) newTablet(opts *Options, t *topodatapb.Tablet) *explainTab
 	tsv.StartService(&target, dbcfgs, nil /* mysqld */)
 
 	// clear all the schema initialization queries out of the tablet
-	// to avoid clutttering the output
+	// to avoid cluttering the output
 	tablet.mysqlQueries = nil
 
 	return &tablet
@@ -300,6 +300,15 @@ func newTabletEnvironment(ddls []sqlparser.DDLStatement, opts *Options) (*tablet
 				{sqltypes.NewVarBinary("STRICT_TRANS_TABLES")},
 			},
 		},
+		"select @@session.sql_mode as sql_mode": {
+			Fields: []*querypb.Field{{
+				Name: "sql_mode",
+				Type: sqltypes.VarChar,
+			}},
+			Rows: [][]sqltypes.Value{
+				{sqltypes.NewVarBinary("STRICT_TRANS_TABLES")},
+			},
+		},
 		"select @@autocommit": {
 			Fields: []*querypb.Field{{
 				Type: sqltypes.Uint64,

diff --git a/go/vt/vttablet/tabletserver/schema/engine.go b/go/vt/vttablet/tabletserver/schema/engine.go
@@ -147,7 +147,7 @@ func (se *Engine) syncSidecarDB(ctx context.Context, conn *dbconnpool.DBConnecti
 				return nil, err
 			}
 		}
-		return conn.ExecuteFetch(query, maxRows, false)
+		return conn.ExecuteFetch(query, maxRows, true)
 	}
 	if err := sidecardb.Init(ctx, exec); err != nil {
 		log.Errorf("Error in sidecardb.Init: %+v", err)

diff --git a/go/vt/vttest/local_cluster.go b/go/vt/vttest/local_cluster.go
@@ -506,8 +506,7 @@ func (db *LocalCluster) createVTSchema() error {
 				return nil, err
 			}
 		}
-		err := db.Execute([]string{query}, "")
-		return &sqltypes.Result{}, err
+		return db.ExecuteFetch(query, "")
 	}
 
 	if err := sidecardb.Init(context.Background(), sidecardbExec); err != nil {
@@ -564,6 +563,21 @@ func (db *LocalCluster) Execute(sql []string, dbname string) error {
 	return err
 }
 
+// ExecuteFetch runs a SQL statement on the MySQL instance backing
+// this local cluster and returns the result.
+func (db *LocalCluster) ExecuteFetch(sql string, dbname string) (*sqltypes.Result, error) {
+	params := db.mysql.Params(dbname)
+	conn, err := mysql.Connect(context.Background(), &params)
+	if err != nil {
+		return nil, err
+	}
+	defer conn.Close()
+
+	log.Infof("ExecuteFetch(%s): \"%s\"", dbname, sql)
+	rs, err := conn.ExecuteFetch(sql, -1, true)
+	return rs, err
+}
+
 // Query runs a  SQL query on the MySQL instance backing this local cluster and returns
 // its result. This is provided for debug/introspection purposes;
 // normal cluster access should be performed through the Vitess GRPC interface.