From ac5aa111fe097ddd095595bca05a9f35cecb53cf Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Thu, 10 Aug 2023 12:01:49 -0400 Subject: [PATCH] Add invariants for sql_mode and foreign_key_check queries Signed-off-by: Matt Lord --- go/vt/binlog/binlogplayer/mock_dbclient.go | 30 ++++++++++++++----- .../tabletmanager/rpc_vreplication_test.go | 22 -------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/go/vt/binlog/binlogplayer/mock_dbclient.go b/go/vt/binlog/binlogplayer/mock_dbclient.go index a30ccdead7a..f8c97fff879 100644 --- a/go/vt/binlog/binlogplayer/mock_dbclient.go +++ b/go/vt/binlog/binlogplayer/mock_dbclient.go @@ -39,7 +39,6 @@ type MockDBClient struct { currentResult int done chan struct{} invariants map[string]*sqltypes.Result - ignored map[string]struct{} } type mockExpect struct { @@ -59,8 +58,29 @@ func NewMockDBClient(t *testing.T) *MockDBClient { "CREATE TABLE IF NOT EXISTS _vt.vreplication_log": {}, "select id, type, state, message from _vt.vreplication_log": {}, "insert into _vt.vreplication_log": {}, + // The following statements don't have a deterministic order as they are + // executed in the normal program flow, but ALSO done in a defer as a protective + // measure as they are resetting the values back to the original one. This also + // means that the values they set are based on the session defaults, which can + // change. So we make these invariants for unit test stability. + "select @@foreign_key_checks": sqltypes.MakeTestResult( + sqltypes.MakeTestFields( + "@@foreign_key_checks", + "int64", + ), + "1", + ), + "set @@session.foreign_key_checks": {}, + "set foreign_key_checks": {}, + "select @@session.sql_mode": sqltypes.MakeTestResult( + sqltypes.MakeTestFields( + "sql_mode", "varchar", + ), + "ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION", + ), + "set @@session.sql_mode": {}, + "set sql_mode": {}, }, - ignored: map[string]struct{}{}, } } @@ -159,12 +179,8 @@ func (dc *MockDBClient) ExecuteFetch(query string, maxrows int) (qr *sqltypes.Re dc.t.Helper() dc.t.Logf("DBClient query: %v", query) - if _, ok := dc.ignored[query]; ok { - return qr, nil - } - for q, result := range dc.invariants { - if strings.Contains(query, q) { + if strings.Contains(strings.ToLower(query), strings.ToLower(q)) { return result, nil } } diff --git a/go/vt/vttablet/tabletmanager/rpc_vreplication_test.go b/go/vt/vttablet/tabletmanager/rpc_vreplication_test.go index 8c8e5c0e37b..a7fbb59c9d8 100644 --- a/go/vt/vttablet/tabletmanager/rpc_vreplication_test.go +++ b/go/vt/vttablet/tabletmanager/rpc_vreplication_test.go @@ -57,13 +57,6 @@ const ( getAutoIncrementStep = "select @@session.auto_increment_increment" setSessionTZ = "set @@session.time_zone = '+00:00'" setNames = "set names 'binary'" - setSQLMode = "set @@session.sql_mode = CONCAT(@@session.sql_mode, ',NO_AUTO_VALUE_ON_ZERO')" - setPermissiveSQLMode = "SET @@session.sql_mode='NO_AUTO_VALUE_ON_ZERO'" - setStrictSQLMode = "SET @@session.sql_mode='ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'" - getSQLMode = "SELECT @@session.sql_mode AS sql_mode" - getFKChecks = "select @@foreign_key_checks" - enableFKChecks = "set foreign_key_checks=1" - sqlMode = "ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION" getBinlogRowImage = "select @@binlog_row_image" insertStreamsCreatedLog = "insert into _vt.vreplication_log(vrepl_id, type, state, message) values(1, 'Stream Created', '', '%s'" getVReplicationRecord = "select * from _vt.vreplication where id = 1" @@ -323,11 +316,6 @@ func TestMoveTables(t *testing.T) { ftc.vrdbClient.ExpectRequest(`update _vt.vreplication set message='Picked source tablet: cell:\"zone1\" uid:200' where id=1`, &sqltypes.Result{}, nil) ftc.vrdbClient.ExpectRequest(setSessionTZ, &sqltypes.Result{}, nil) ftc.vrdbClient.ExpectRequest(setNames, &sqltypes.Result{}, nil) - ftc.vrdbClient.ExpectRequest(setSQLMode, &sqltypes.Result{}, nil) - ftc.vrdbClient.ExpectRequest(getSQLMode, sqltypes.MakeTestResult( - sqltypes.MakeTestFields("sql_mode", "varchar"), - sqlMode, - ), nil) ftc.vrdbClient.ExpectRequest(getWorkflowState, sqltypes.MakeTestResult( sqltypes.MakeTestFields( "pos|stop_pos|max_tps|max_replication_lag|state|workflow_type|workflow|workflow_sub_type|defer_secondary_keys", @@ -342,14 +330,6 @@ func TestMoveTables(t *testing.T) { ), "1", ), nil) - ftc.vrdbClient.ExpectRequest(setPermissiveSQLMode, &sqltypes.Result{}, nil) - ftc.vrdbClient.ExpectRequest(getFKChecks, sqltypes.MakeTestResult( - sqltypes.MakeTestFields( - "@@foreign_key_checks", - "int64", - ), - "1", - ), nil) ftc.vrdbClient.ExpectRequest(getWorkflowState, sqltypes.MakeTestResult( sqltypes.MakeTestFields( "pos|stop_pos|max_tps|max_replication_lag|state|workflow_type|workflow|workflow_sub_type|defer_secondary_keys", @@ -371,8 +351,6 @@ func TestMoveTables(t *testing.T) { ), "FULL", ), nil) - ftc.vrdbClient.ExpectRequest(enableFKChecks, &sqltypes.Result{}, nil) - ftc.vrdbClient.ExpectRequest(setStrictSQLMode, &sqltypes.Result{}, nil) ftc.vrdbClient.ExpectRequest(fmt.Sprintf(insertStreamsCreatedLog, bls), &sqltypes.Result{}, nil) tenv.tmc.setVReplicationExecResults(ftc.tablet, fmt.Sprintf(getWorkflow, targetKs, wf),