From 2809eda43c90d77e0fe7eb5c29ebc0a238c20dd1 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 6 Nov 2023 16:20:12 +0530 Subject: [PATCH 01/25] feat: make foreign_key_checks system aware vitess variable Signed-off-by: Manan Gupta --- go/vt/sysvars/sysvars.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/vt/sysvars/sysvars.go b/go/vt/sysvars/sysvars.go index 98da8ff07b7..945a60ae965 100644 --- a/go/vt/sysvars/sysvars.go +++ b/go/vt/sysvars/sysvars.go @@ -71,6 +71,7 @@ var ( TxReadOnly = SystemVariable{Name: "tx_read_only", IsBoolean: true, Default: off} Workload = SystemVariable{Name: "workload", IdentifierAsString: true} QueryTimeout = SystemVariable{Name: "query_timeout"} + ForeignKeyChecks = SystemVariable{Name: "foreign_key_checks", IsBoolean: true, SupportSetVar: true} // Online DDL DDLStrategy = SystemVariable{Name: "ddl_strategy", IdentifierAsString: true} @@ -104,6 +105,7 @@ var ( ReadAfterWriteTimeOut, SessionTrackGTIDs, QueryTimeout, + ForeignKeyChecks, } ReadOnly = []SystemVariable{ @@ -186,7 +188,6 @@ var ( {Name: "end_markers_in_json", IsBoolean: true, SupportSetVar: true}, {Name: "eq_range_index_dive_limit", SupportSetVar: true}, {Name: "explicit_defaults_for_timestamp"}, - {Name: "foreign_key_checks", IsBoolean: true, SupportSetVar: true}, {Name: "group_concat_max_len", SupportSetVar: true}, {Name: "information_schema_stats_expiry"}, {Name: "max_heap_table_size", SupportSetVar: true}, From 185b4ea7681a1d7bf196b140ff538e26973ad2d7 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 6 Nov 2023 16:22:31 +0530 Subject: [PATCH 02/25] test: add test for foreign-key-checks being marked as required in needs bind vars Signed-off-by: Manan Gupta --- go/vt/sqlparser/ast_rewriting_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/go/vt/sqlparser/ast_rewriting_test.go b/go/vt/sqlparser/ast_rewriting_test.go index 2ed92201296..60ac8827490 100644 --- a/go/vt/sqlparser/ast_rewriting_test.go +++ b/go/vt/sqlparser/ast_rewriting_test.go @@ -37,12 +37,12 @@ type testCaseSysVar struct { } type myTestCase struct { - in, expected string - liid, db, foundRows, rowCount, rawGTID, rawTimeout, sessTrackGTID bool - ddlStrategy, migrationContext, sessionUUID, sessionEnableSystemSettings bool - udv int - autocommit, clientFoundRows, skipQueryPlanCache, socket, queryTimeout bool - sqlSelectLimit, transactionMode, workload, version, versionComment bool + in, expected string + liid, db, foundRows, rowCount, rawGTID, rawTimeout, sessTrackGTID bool + ddlStrategy, migrationContext, sessionUUID, sessionEnableSystemSettings bool + udv int + autocommit, foreignKeyChecks, clientFoundRows, skipQueryPlanCache, socket, queryTimeout bool + sqlSelectLimit, transactionMode, workload, version, versionComment bool } func TestRewrites(in *testing.T) { @@ -296,6 +296,7 @@ func TestRewrites(in *testing.T) { in: "SHOW VARIABLES", expected: "SHOW VARIABLES", autocommit: true, + foreignKeyChecks: true, clientFoundRows: true, skipQueryPlanCache: true, sqlSelectLimit: true, @@ -316,6 +317,7 @@ func TestRewrites(in *testing.T) { in: "SHOW GLOBAL VARIABLES", expected: "SHOW GLOBAL VARIABLES", autocommit: true, + foreignKeyChecks: true, clientFoundRows: true, skipQueryPlanCache: true, sqlSelectLimit: true, @@ -362,6 +364,7 @@ func TestRewrites(in *testing.T) { assert.Equal(tc.rowCount, result.NeedsFuncResult(RowCountName), "should need row count") assert.Equal(tc.udv, len(result.NeedUserDefinedVariables), "count of user defined variables") assert.Equal(tc.autocommit, result.NeedsSysVar(sysvars.Autocommit.Name), "should need :__vtautocommit") + assert.Equal(tc.foreignKeyChecks, result.NeedsSysVar(sysvars.ForeignKeyChecks.Name), "should need :__vtforeignKeyChecks") assert.Equal(tc.clientFoundRows, result.NeedsSysVar(sysvars.ClientFoundRows.Name), "should need :__vtclientFoundRows") assert.Equal(tc.skipQueryPlanCache, result.NeedsSysVar(sysvars.SkipQueryPlanCache.Name), "should need :__vtskipQueryPlanCache") assert.Equal(tc.sqlSelectLimit, result.NeedsSysVar(sysvars.SQLSelectLimit.Name), "should need :__vtsqlSelectLimit") From 39e44ac9eaf1038871cc0d2408b42623c9a9f485 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 6 Nov 2023 16:23:24 +0530 Subject: [PATCH 03/25] feat: add function for getting mysql set var value for the given key Signed-off-by: Manan Gupta --- go/vt/sqlparser/comments.go | 106 +++++++++++++++++++++++++++++++ go/vt/sqlparser/comments_test.go | 44 +++++++++++++ 2 files changed, 150 insertions(+) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 84b73f8e81c..a38ab0e4ccd 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -60,6 +60,9 @@ const ( // MaxPriorityValue specifies the maximum value allowed for the priority query directive. Valid priority values are // between zero and MaxPriorityValue. MaxPriorityValue = 100 + + // OptimizerHintSetVar is the optimizer hint used in MySQL to set the value of a specific session variable for a query. + OptimizerHintSetVar = "SET_VAR" ) var ErrInvalidPriority = vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Invalid priority value specified in query") @@ -266,6 +269,109 @@ func (c *ParsedComments) Directives() *CommentDirectives { return c._directives } +// GetMySQLSetVarValue gets the value of the given variable if it is part of a /*+ SET_VAR() */ MySQL optimizer hint. +func (c *ParsedComments) GetMySQLSetVarValue(key string) string { + if c == nil { + return "" + } + for _, commentStr := range c.comments { + if commentStr[0:3] != queryOptimizerPrefix { + continue + } + + pos := 4 + for pos < len(commentStr) { + finalPos, ohNameStart, ohNameEnd, ohContentStart, ohContentEnd := getOptimizerHint(pos, commentStr) + pos = finalPos + 1 + if ohContentEnd == -1 { + break + } + ohName := commentStr[ohNameStart:ohNameEnd] + ohContent := commentStr[ohContentStart:ohContentEnd] + if strings.EqualFold(strings.TrimSpace(ohName), OptimizerHintSetVar) { + setVarName, setVarValue, isValid := strings.Cut(ohContent, "=") + if !isValid { + continue + } + if strings.EqualFold(strings.TrimSpace(setVarName), key) { + return strings.TrimSpace(setVarValue) + } + } + } + + return "" + } + return "" +} + +func getOptimizerHint(initialPos int, commentStr string) (pos int, ohNameStart int, ohNameEnd int, ohContentStart int, ohContentEnd int) { + ohContentEnd = -1 + pos = skipBlanks(initialPos, commentStr) + ohNameStart = pos + pos++ + for pos < len(commentStr) { + if commentStr[pos] == ' ' || commentStr[pos] == '(' { + break + } + pos++ + } + ohNameEnd = pos + pos = skipBlanks(pos, commentStr) + if pos >= len(commentStr) || commentStr[pos] != '(' { + return + } + pos++ + ohContentStart = pos + pos = skipUntilParanthesisEnd(pos, commentStr) + ohContentEnd = pos + return +} + +func skipUntilParanthesisEnd(pos int, commentStr string) int { + stack := []byte{'('} + for pos < len(commentStr) { + switch commentStr[pos] { + case '(': + if stack[len(stack)-1] == '(' { + stack = append(stack, '(') + } + case ')': + if stack[len(stack)-1] == '(' { + stack = stack[:len(stack)-1] + } + case '\'': + if stack[len(stack)-1] == '\'' { + stack = stack[:len(stack)-1] + } else if stack[len(stack)-1] != '"' { + stack = append(stack, '\'') + } + case '"': + if stack[len(stack)-1] == '"' { + stack = stack[:len(stack)-1] + } else if stack[len(stack)-1] != '\'' { + stack = append(stack, '"') + } + } + if len(stack) == 0 { + return pos + } + pos++ + } + + return pos +} + +func skipBlanks(pos int, commentStr string) int { + for pos < len(commentStr) { + if commentStr[pos] == ' ' { + pos++ + continue + } + break + } + return pos +} + func (c *ParsedComments) Length() int { if c == nil { return 0 diff --git a/go/vt/sqlparser/comments_test.go b/go/vt/sqlparser/comments_test.go index b3c1bf9fec8..27759c90424 100644 --- a/go/vt/sqlparser/comments_test.go +++ b/go/vt/sqlparser/comments_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/assert" querypb "vitess.io/vitess/go/vt/proto/query" + "vitess.io/vitess/go/vt/sysvars" ) func TestSplitComments(t *testing.T) { @@ -551,3 +552,46 @@ func TestGetPriorityFromStatement(t *testing.T) { }) } } + +// TestGetMySQLSetVarValue tests the functionality of GetMySQLSetVarValue +func TestGetMySQLSetVarValue(t *testing.T) { + tests := []struct { + name string + comments []string + valToFind string + want string + }{ + { + name: "SET_VAR clause in the middle", + comments: []string{"/*+ NO_RANGE_OPTIMIZATION(t3 PRIMARY, f2_idx) SET_VAR(foreign_key_checks=OFF) NO_ICP(t1, t2) */"}, + valToFind: sysvars.ForeignKeyChecks.Name, + want: "OFF", + }, + { + name: "Single SET_VAR clause", + comments: []string{"/*+ SET_VAR(sort_buffer_size = 16M) */"}, + valToFind: "sort_buffer_size", + want: "16M", + }, + { + name: "Multiple SET_VAR clauses", + comments: []string{"/*+ SET_VAR(sort_buffer_size = 16M) */", "/*+ SET_VAR(optimizer_switch = 'mrr_cost_b(ased=of\"f') */", "/*+ SET_VAR( foReiGn_key_checks = On) */"}, + valToFind: sysvars.ForeignKeyChecks.Name, + want: "", + }, + { + name: "Verify casing", + comments: []string{"/*+ SET_VAR(optimizer_switch = 'mrr_cost_b(ased=of\"f') SET_VAR( foReiGn_key_checks = On) */"}, + valToFind: sysvars.ForeignKeyChecks.Name, + want: "On", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &ParsedComments{ + comments: tt.comments, + } + assert.Equal(t, tt.want, c.GetMySQLSetVarValue(tt.valToFind)) + }) + } +} From ecdf5948bd311e7507517974ad8d3ca7e35c2f85 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 6 Nov 2023 16:24:22 +0530 Subject: [PATCH 04/25] feat: add function to set the foreign key checks on the vtgate session Signed-off-by: Manan Gupta --- go/vt/vtgate/engine/fake_vcursor_test.go | 8 ++++++++ go/vt/vtgate/engine/primitive.go | 1 + go/vt/vtgate/engine/set.go | 2 ++ go/vt/vtgate/vcursor_impl.go | 11 +++++++++++ 4 files changed, 22 insertions(+) diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go index c2418c73560..b3370ffadad 100644 --- a/go/vt/vtgate/engine/fake_vcursor_test.go +++ b/go/vt/vtgate/engine/fake_vcursor_test.go @@ -267,6 +267,10 @@ func (t *noopVCursor) SetAutocommit(context.Context, bool) error { panic("implement me") } +func (t *noopVCursor) SetSessionForeignKeyChecks(ctx context.Context, foreignKeyChecks bool) error { + panic("implement me") +} + func (t *noopVCursor) SetClientFoundRows(context.Context, bool) error { panic("implement me") } @@ -735,6 +739,10 @@ func (f *loggingVCursor) SetAutocommit(context.Context, bool) error { panic("implement me") } +func (f *loggingVCursor) SetSessionForeignKeyChecks(ctx context.Context, foreignKeyChecks bool) error { + panic("implement me") +} + func (f *loggingVCursor) SetClientFoundRows(context.Context, bool) error { panic("implement me") } diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go index a3a37f97fe4..dc1259e2267 100644 --- a/go/vt/vtgate/engine/primitive.go +++ b/go/vt/vtgate/engine/primitive.go @@ -153,6 +153,7 @@ type ( SetAutocommit(ctx context.Context, autocommit bool) error SetClientFoundRows(context.Context, bool) error + SetSessionForeignKeyChecks(ctx context.Context, autocommit bool) error SetSkipQueryPlanCache(context.Context, bool) error SetSQLSelectLimit(int64) error SetTransactionMode(vtgatepb.TransactionMode) diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go index df56fc04ed2..bd81146b371 100644 --- a/go/vt/vtgate/engine/set.go +++ b/go/vt/vtgate/engine/set.go @@ -450,6 +450,8 @@ func (svss *SysVarSetAware) Execute(ctx context.Context, vcursor VCursor, env *e switch svss.Name { case sysvars.Autocommit.Name: err = svss.setBoolSysVar(ctx, env, vcursor.Session().SetAutocommit) + case sysvars.ForeignKeyChecks.Name: + err = svss.setBoolSysVar(ctx, env, vcursor.Session().SetSessionForeignKeyChecks) case sysvars.ClientFoundRows.Name: err = svss.setBoolSysVar(ctx, env, vcursor.Session().SetClientFoundRows) case sysvars.SkipQueryPlanCache.Name: diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index 0e89d6fbc95..94ad2220a27 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -28,6 +28,7 @@ import ( "github.com/google/uuid" "vitess.io/vitess/go/mysql/sqlerror" + "vitess.io/vitess/go/vt/sysvars" "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/sqltypes" @@ -820,6 +821,16 @@ func (vc *vcursorImpl) SetAutocommit(ctx context.Context, autocommit bool) error return nil } +// SetSessionForeignKeyChecks implements the SessionActions interface +func (vc *vcursorImpl) SetSessionForeignKeyChecks(ctx context.Context, foreignKeyChecks bool) error { + if foreignKeyChecks { + vc.safeSession.SetSystemVariable(sysvars.ForeignKeyChecks.Name, "1") + } else { + vc.safeSession.SetSystemVariable(sysvars.ForeignKeyChecks.Name, "0") + } + return nil +} + // SetQueryTimeout implements the SessionActions interface func (vc *vcursorImpl) SetQueryTimeout(maxExecutionTime int64) { vc.safeSession.QueryTimeout = maxExecutionTime From a4eda3f710bd73d181f846d83e227e33900f7529 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 6 Nov 2023 21:54:51 +0530 Subject: [PATCH 05/25] feat: add function to set a variable in SET_VAR optimizer hint Signed-off-by: Manan Gupta --- go/vt/sqlparser/comments.go | 65 ++++++++++++++++++++++++++++ go/vt/sqlparser/comments_test.go | 74 ++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index a38ab0e4ccd..a840b05308d 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -17,11 +17,13 @@ limitations under the License. package sqlparser import ( + "fmt" "strconv" "strings" "unicode" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/sysvars" "vitess.io/vitess/go/vt/vterrors" querypb "vitess.io/vitess/go/vt/proto/query" @@ -304,6 +306,54 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { return "" } +// SetMySQLSetVarValue updates or sets the value of the given variable as part of a /*+ SET_VAR() */ MySQL optimizer hint. +func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) { + if c == nil { + c = &ParsedComments{ + comments: Comments{}, + } + } + for idx, commentStr := range c.comments { + if commentStr[0:3] != queryOptimizerPrefix { + continue + } + + finalComment := "/*+" + keyPresent := false + pos := 4 + for pos < len(commentStr) { + finalPos, ohNameStart, ohNameEnd, ohContentStart, ohContentEnd := getOptimizerHint(pos, commentStr) + pos = finalPos + 1 + if ohContentEnd == -1 { + break + } + ohName := commentStr[ohNameStart:ohNameEnd] + ohContent := commentStr[ohContentStart:ohContentEnd] + if strings.EqualFold(strings.TrimSpace(ohName), OptimizerHintSetVar) { + setVarName, _, isValid := strings.Cut(ohContent, "=") + if !isValid || !strings.EqualFold(strings.TrimSpace(setVarName), key) { + finalComment += fmt.Sprintf(" %v(%v)", ohName, ohContent) + continue + } + if strings.EqualFold(strings.TrimSpace(setVarName), key) { + keyPresent = true + finalComment += fmt.Sprintf(" %v(%v=%v)", ohName, strings.TrimSpace(setVarName), value) + } + } else { + finalComment += fmt.Sprintf(" %v(%v)", ohName, ohContent) + } + } + if !keyPresent { + finalComment += fmt.Sprintf(" %v(%v=%v)", OptimizerHintSetVar, key, value) + } + + finalComment += " */" + c.comments[idx] = finalComment + return + } + c.comments = append(c.comments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) +} + func getOptimizerHint(initialPos int, commentStr string) (pos int, ohNameStart int, ohNameEnd int, ohContentStart int, ohContentEnd int) { ohContentEnd = -1 pos = skipBlanks(initialPos, commentStr) @@ -455,6 +505,21 @@ func AllowScatterDirective(stmt Statement) bool { return checkDirective(stmt, DirectiveAllowScatter) } +// ForeignKeyChecksState returns the state of foreign_key_checks variable if it is part of a SET_VAR optimizer hint in the comments. +func ForeignKeyChecksState(stmt Statement) FkChecksState { + cmt, ok := stmt.(Commented) + if ok { + fkChecksVal := cmt.GetParsedComments().GetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name) + switch strings.ToLower(fkChecksVal) { + case "on", "1": + return FkChecksOn + case "off", "0": + return FkChecksOff + } + } + return FkChecksUnspecified +} + func checkDirective(stmt Statement, key string) bool { cmt, ok := stmt.(Commented) if ok { diff --git a/go/vt/sqlparser/comments_test.go b/go/vt/sqlparser/comments_test.go index 27759c90424..e5bcda1d4ba 100644 --- a/go/vt/sqlparser/comments_test.go +++ b/go/vt/sqlparser/comments_test.go @@ -573,6 +573,12 @@ func TestGetMySQLSetVarValue(t *testing.T) { valToFind: "sort_buffer_size", want: "16M", }, + { + name: "No comments", + comments: nil, + valToFind: "sort_buffer_size", + want: "", + }, { name: "Multiple SET_VAR clauses", comments: []string{"/*+ SET_VAR(sort_buffer_size = 16M) */", "/*+ SET_VAR(optimizer_switch = 'mrr_cost_b(ased=of\"f') */", "/*+ SET_VAR( foReiGn_key_checks = On) */"}, @@ -585,6 +591,12 @@ func TestGetMySQLSetVarValue(t *testing.T) { valToFind: sysvars.ForeignKeyChecks.Name, want: "On", }, + { + name: "Leading comment is a normal comment", + comments: []string{"/* This is a normal comment */", "/*+ MAX_EXECUTION_TIME(1000) SET_VAR( foreign_key_checks = 1) */"}, + valToFind: sysvars.ForeignKeyChecks.Name, + want: "1", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -595,3 +607,65 @@ func TestGetMySQLSetVarValue(t *testing.T) { }) } } + +func TestSetMySQLSetVarValue(t *testing.T) { + tests := []struct { + name string + comments []string + key string + value string + commentsWanted Comments + }{ + { + name: "SET_VAR clause in the middle", + comments: []string{"/*+ NO_RANGE_OPTIMIZATION(t3 PRIMARY, f2_idx) SET_VAR(foreign_key_checks=OFF) NO_ICP(t1, t2) */"}, + key: sysvars.ForeignKeyChecks.Name, + value: "On", + commentsWanted: []string{"/*+ NO_RANGE_OPTIMIZATION(t3 PRIMARY, f2_idx) SET_VAR(foreign_key_checks=On) NO_ICP(t1, t2) */"}, + }, + { + name: "Single SET_VAR clause", + comments: []string{"/*+ SET_VAR(sort_buffer_size = 16M) */"}, + key: "sort_buffer_size", + value: "1Mb", + commentsWanted: []string{"/*+ SET_VAR(sort_buffer_size=1Mb) */"}, + }, + { + name: "No comments", + comments: nil, + key: "sort_buffer_size", + value: "13M", + commentsWanted: []string{"/*+ SET_VAR(sort_buffer_size=13M) */"}, + }, + { + name: "Multiple SET_VAR clauses", + comments: []string{"/*+ SET_VAR(sort_buffer_size = 16M) */", "/*+ SET_VAR(optimizer_switch = 'mrr_cost_b(ased=of\"f') */", "/*+ SET_VAR( foReiGn_key_checks = On) */"}, + key: sysvars.ForeignKeyChecks.Name, + value: "1", + commentsWanted: []string{"/*+ SET_VAR(sort_buffer_size = 16M) SET_VAR(foreign_key_checks=1) */", "/*+ SET_VAR(optimizer_switch = 'mrr_cost_b(ased=of\"f') */", "/*+ SET_VAR( foReiGn_key_checks = On) */"}, + }, + { + name: "Verify casing", + comments: []string{"/*+ SET_VAR(optimizer_switch = 'mrr_cost_b(ased=of\"f') SET_VAR( foReiGn_key_checks = On) */"}, + key: sysvars.ForeignKeyChecks.Name, + value: "off", + commentsWanted: []string{"/*+ SET_VAR(optimizer_switch = 'mrr_cost_b(ased=of\"f') SET_VAR(foReiGn_key_checks=off) */"}, + }, + { + name: "Leading comment is a normal comment", + comments: []string{"/* This is a normal comment */", "/*+ MAX_EXECUTION_TIME(1000) SET_VAR( foreign_key_checks = 1) */"}, + key: sysvars.ForeignKeyChecks.Name, + value: "Off", + commentsWanted: []string{"/* This is a normal comment */", "/*+ MAX_EXECUTION_TIME(1000) SET_VAR(foreign_key_checks=Off) */"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &ParsedComments{ + comments: tt.comments, + } + c.SetMySQLSetVarValue(tt.key, tt.value) + require.EqualValues(t, tt.commentsWanted, c.comments) + }) + } +} From cc274a7af0484f90f5d71a5dc2e7751d345733e9 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 6 Nov 2023 22:06:16 +0530 Subject: [PATCH 06/25] feat: read foreign_key_checks from comments and rewrite to appropriate value Signed-off-by: Manan Gupta --- go/vt/sqlparser/ast_rewriting.go | 33 ++++++++++++++------- go/vt/sqlparser/ast_rewriting_test.go | 7 +++-- go/vt/sqlparser/constants.go | 9 ++++++ go/vt/sqlparser/normalizer_test.go | 2 ++ go/vt/vtgate/executor.go | 2 ++ go/vt/vtgate/planbuilder/builder.go | 2 +- go/vt/vtgate/planbuilder/simplifier_test.go | 8 ++--- go/vt/vtgate/vcursor_impl.go | 24 +++++++++++++++ 8 files changed, 69 insertions(+), 18 deletions(-) diff --git a/go/vt/sqlparser/ast_rewriting.go b/go/vt/sqlparser/ast_rewriting.go index 45711f8d535..00b0780341d 100644 --- a/go/vt/sqlparser/ast_rewriting.go +++ b/go/vt/sqlparser/ast_rewriting.go @@ -51,6 +51,7 @@ func PrepareAST( selectLimit int, setVarComment string, sysVars map[string]string, + fkChecksState FkChecksState, views VSchemaViews, ) (*RewriteASTResult, error) { if parameterize { @@ -59,7 +60,7 @@ func PrepareAST( return nil, err } } - return RewriteAST(in, keyspace, selectLimit, setVarComment, sysVars, views) + return RewriteAST(in, keyspace, selectLimit, setVarComment, sysVars, fkChecksState, views) } // RewriteAST rewrites the whole AST, replacing function calls and adding column aliases to queries. @@ -70,9 +71,10 @@ func RewriteAST( selectLimit int, setVarComment string, sysVars map[string]string, + fkChecksState FkChecksState, views VSchemaViews, ) (*RewriteASTResult, error) { - er := newASTRewriter(keyspace, selectLimit, setVarComment, sysVars, views) + er := newASTRewriter(keyspace, selectLimit, setVarComment, sysVars, fkChecksState, views) er.shouldRewriteDatabaseFunc = shouldRewriteDatabaseFunc(in) result := SafeRewrite(in, er.rewriteDown, er.rewriteUp) if er.err != nil { @@ -121,16 +123,18 @@ type astRewriter struct { keyspace string selectLimit int setVarComment string + fkChecksState FkChecksState sysVars map[string]string views VSchemaViews } -func newASTRewriter(keyspace string, selectLimit int, setVarComment string, sysVars map[string]string, views VSchemaViews) *astRewriter { +func newASTRewriter(keyspace string, selectLimit int, setVarComment string, sysVars map[string]string, fkChecksState FkChecksState, views VSchemaViews) *astRewriter { return &astRewriter{ bindVars: &BindVarNeeds{}, keyspace: keyspace, selectLimit: selectLimit, setVarComment: setVarComment, + fkChecksState: fkChecksState, sysVars: sysVars, views: views, } @@ -154,7 +158,7 @@ const ( ) func (er *astRewriter) rewriteAliasedExpr(node *AliasedExpr) (*BindVarNeeds, error) { - inner := newASTRewriter(er.keyspace, er.selectLimit, er.setVarComment, er.sysVars, er.views) + inner := newASTRewriter(er.keyspace, er.selectLimit, er.setVarComment, er.sysVars, FkChecksUnspecified, er.views) inner.shouldRewriteDatabaseFunc = er.shouldRewriteDatabaseFunc tmp := SafeRewrite(node.Expr, inner.rewriteDown, inner.rewriteUp) newExpr, ok := tmp.(Expr) @@ -177,13 +181,22 @@ func (er *astRewriter) rewriteDown(node SQLNode, _ SQLNode) bool { func (er *astRewriter) rewriteUp(cursor *Cursor) bool { // Add SET_VAR comment to this node if it supports it and is needed - if supportOptimizerHint, supportsOptimizerHint := cursor.Node().(SupportOptimizerHint); supportsOptimizerHint && er.setVarComment != "" { - newComments, err := supportOptimizerHint.GetParsedComments().AddQueryHint(er.setVarComment) - if err != nil { - er.err = err - return false + if supportOptimizerHint, supportsOptimizerHint := cursor.Node().(SupportOptimizerHint); supportsOptimizerHint { + if er.setVarComment != "" { + newComments, err := supportOptimizerHint.GetParsedComments().AddQueryHint(er.setVarComment) + if err != nil { + er.err = err + return false + } + supportOptimizerHint.SetComments(newComments) + } + if er.fkChecksState != FkChecksUnspecified { + value := "On" + if er.fkChecksState == FkChecksOff { + value = "Off" + } + supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, value) } - supportOptimizerHint.SetComments(newComments) } switch node := cursor.Node().(type) { diff --git a/go/vt/sqlparser/ast_rewriting_test.go b/go/vt/sqlparser/ast_rewriting_test.go index 60ac8827490..091546e8b6c 100644 --- a/go/vt/sqlparser/ast_rewriting_test.go +++ b/go/vt/sqlparser/ast_rewriting_test.go @@ -348,6 +348,7 @@ func TestRewrites(in *testing.T) { SQLSelectLimitUnset, "", nil, + FkChecksUnspecified, &fakeViews{}, ) require.NoError(err) @@ -439,7 +440,7 @@ func TestRewritesWithSetVarComment(in *testing.T) { stmt, err := Parse(tc.in) require.NoError(err) - result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, tc.setVarComment, nil, &fakeViews{}) + result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, tc.setVarComment, nil, FkChecksUnspecified, &fakeViews{}) require.NoError(err) expected, err := Parse(tc.expected) @@ -487,7 +488,7 @@ func TestRewritesSysVar(in *testing.T) { stmt, err := Parse(tc.in) require.NoError(err) - result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, "", tc.sysVar, &fakeViews{}) + result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, "", tc.sysVar, FkChecksUnspecified, &fakeViews{}) require.NoError(err) expected, err := Parse(tc.expected) @@ -537,7 +538,7 @@ func TestRewritesWithDefaultKeyspace(in *testing.T) { stmt, err := Parse(tc.in) require.NoError(err) - result, err := RewriteAST(stmt, "sys", SQLSelectLimitUnset, "", nil, &fakeViews{}) + result, err := RewriteAST(stmt, "sys", SQLSelectLimitUnset, "", nil, FkChecksUnspecified, &fakeViews{}) require.NoError(err) expected, err := Parse(tc.expected) diff --git a/go/vt/sqlparser/constants.go b/go/vt/sqlparser/constants.go index 3848c53f3e0..8a8a673a754 100644 --- a/go/vt/sqlparser/constants.go +++ b/go/vt/sqlparser/constants.go @@ -1070,3 +1070,12 @@ const ( IndexTypeSpatial IndexTypeFullText ) + +// FkChecksState is the enum to store the value of `foreign_key_checks` MySQL variable. +type FkChecksState int + +const ( + FkChecksUnspecified FkChecksState = iota + FkChecksOn + FkChecksOff +) diff --git a/go/vt/sqlparser/normalizer_test.go b/go/vt/sqlparser/normalizer_test.go index f4f5f89d99d..ebc726b54ae 100644 --- a/go/vt/sqlparser/normalizer_test.go +++ b/go/vt/sqlparser/normalizer_test.go @@ -565,6 +565,7 @@ func BenchmarkNormalizeVTGate(b *testing.B) { SQLSelectLimitUnset, "", nil, /*sysvars*/ + FkChecksUnspecified, nil, /*views*/ ) if err != nil { @@ -855,6 +856,7 @@ func benchmarkNormalization(b *testing.B, sqls []string) { SQLSelectLimitUnset, "", nil, + FkChecksUnspecified, nil, ) if err != nil { diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index c7843e26bbf..d027e420d3e 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -1042,6 +1042,7 @@ func (e *Executor) getPlan( vcursor.SetIgnoreMaxMemoryRows(sqlparser.IgnoreMaxMaxMemoryRowsDirective(stmt)) vcursor.SetConsolidator(sqlparser.Consolidator(stmt)) vcursor.SetWorkloadName(sqlparser.GetWorkloadNameFromStatement(stmt)) + vcursor.UpdateForeignKeyChecksState(sqlparser.ForeignKeyChecksState(stmt)) priority, err := sqlparser.GetPriorityFromStatement(stmt) if err != nil { return nil, err @@ -1066,6 +1067,7 @@ func (e *Executor) getPlan( vcursor.safeSession.getSelectLimit(), setVarComment, vcursor.safeSession.SystemVariables, + vcursor.GetForeignKeyChecksState(), vcursor, ) if err != nil { diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index a67878d7119..0b778914739 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -74,7 +74,7 @@ func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*e if err != nil { return nil, err } - result, err := sqlparser.RewriteAST(stmt, keyspace, sqlparser.SQLSelectLimitUnset, "", nil, vschema) + result, err := sqlparser.RewriteAST(stmt, keyspace, sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, vschema) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/simplifier_test.go b/go/vt/vtgate/planbuilder/simplifier_test.go index 1e106adacc0..5f39894a3ec 100644 --- a/go/vt/vtgate/planbuilder/simplifier_test.go +++ b/go/vt/vtgate/planbuilder/simplifier_test.go @@ -48,7 +48,7 @@ func TestSimplifyBuggyQuery(t *testing.T) { } stmt, reserved, err := sqlparser.Parse2(query) require.NoError(t, err) - rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil) + rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) reservedVars := sqlparser.NewReservedVars("vtg", reserved) simplified := simplifier.SimplifyStatement( @@ -70,7 +70,7 @@ func TestSimplifyPanic(t *testing.T) { } stmt, reserved, err := sqlparser.Parse2(query) require.NoError(t, err) - rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil) + rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) reservedVars := sqlparser.NewReservedVars("vtg", reserved) simplified := simplifier.SimplifyStatement( @@ -100,7 +100,7 @@ func TestUnsupportedFile(t *testing.T) { t.Skip() return } - rewritten, err := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil) + rewritten, err := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) if err != nil { t.Skip() } @@ -134,7 +134,7 @@ func keepSameError(query string, reservedVars *sqlparser.ReservedVars, vschema * if err != nil { panic(err) } - rewritten, _ := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil) + rewritten, _ := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) ast := rewritten.AST _, expected := BuildFromStmt(context.Background(), query, ast, reservedVars, vschema, rewritten.BindVarNeeds, true, true) if expected == nil { diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index 94ad2220a27..3d8d85582a6 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -107,6 +107,7 @@ type vcursorImpl struct { collation collations.ID ignoreMaxMemoryRows bool + fkChecksState sqlparser.FkChecksState vschema *vindexes.VSchema vm VSchemaOperator semTable *semantics.SemTable @@ -1342,3 +1343,26 @@ func (vc *vcursorImpl) CloneForReplicaWarming(ctx context.Context) engine.VCurso return v } + +// UpdateForeignKeyChecksState updates the foreign key checks state of the vcursor. +func (vc *vcursorImpl) UpdateForeignKeyChecksState(fkStateFromQuery sqlparser.FkChecksState) { + vc.fkChecksState = sqlparser.FkChecksUnspecified + if fkStateFromQuery != sqlparser.FkChecksUnspecified { + vc.fkChecksState = fkStateFromQuery + return + } + fkVal, isPresent := vc.safeSession.SystemVariables[sysvars.ForeignKeyChecks.Name] + if isPresent { + switch strings.ToLower(fkVal) { + case "on", "1": + vc.fkChecksState = sqlparser.FkChecksOn + case "off", "0": + vc.fkChecksState = sqlparser.FkChecksOff + } + } +} + +// GetForeignKeyChecksState gets the stored foreign key checks state in the vcursor. +func (vc *vcursorImpl) GetForeignKeyChecksState() sqlparser.FkChecksState { + return vc.fkChecksState +} From d8a6a042eef682100dcff9497bf87562cb7de749 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 6 Nov 2023 22:12:08 +0530 Subject: [PATCH 07/25] feat: add fkChecksState to the plan key Signed-off-by: Manan Gupta --- go/vt/sqlparser/ast_funcs.go | 10 ++++++++++ go/vt/sqlparser/ast_rewriting.go | 6 +----- go/vt/vtgate/vcursor_impl.go | 2 ++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 951d9879bdb..66ba356cf66 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -357,6 +357,16 @@ func (node *ParsedComments) AddQueryHint(queryHint string) (Comments, error) { return newComments, nil } +func (s FkChecksState) String() string { + switch s { + case FkChecksOff: + return "Off" + case FkChecksOn: + return "On" + } + return "" +} + // ParseParams parses the vindex parameter list, pulling out the special-case // "owner" parameter func (node *VindexSpec) ParseParams() (string, map[string]string) { diff --git a/go/vt/sqlparser/ast_rewriting.go b/go/vt/sqlparser/ast_rewriting.go index 00b0780341d..6d0c6eaec85 100644 --- a/go/vt/sqlparser/ast_rewriting.go +++ b/go/vt/sqlparser/ast_rewriting.go @@ -191,11 +191,7 @@ func (er *astRewriter) rewriteUp(cursor *Cursor) bool { supportOptimizerHint.SetComments(newComments) } if er.fkChecksState != FkChecksUnspecified { - value := "On" - if er.fkChecksState == FkChecksOff { - value = "Off" - } - supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, value) + supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, er.fkChecksState.String()) } } diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index 3d8d85582a6..dc10cd2e8d0 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -1084,6 +1084,8 @@ func (vc *vcursorImpl) keyForPlan(ctx context.Context, query string, buf io.Stri _, _ = buf.WriteString(vindexes.TabletTypeSuffix[vc.tabletType]) _, _ = buf.WriteString("+Collate:") _, _ = buf.WriteString(collations.Local().LookupName(vc.collation)) + _, _ = buf.WriteString("+fkChecksState:") + _, _ = buf.WriteString(vc.GetForeignKeyChecksState().String()) if vc.destination != nil { switch vc.destination.(type) { From 48e875c689ae9667667b202270d25db632ee90f6 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 7 Nov 2023 15:46:43 +0530 Subject: [PATCH 08/25] feat: ignore foreign keys planning if fk checks are turned off Signed-off-by: Manan Gupta --- go/test/vschemawrapper/vschema_wrapper.go | 21 ++++++++++++------- go/vt/schemadiff/semantics.go | 4 ++++ .../vtgate/planbuilder/plancontext/vschema.go | 2 ++ go/vt/vtgate/semantics/FakeSI.go | 4 ++++ go/vt/vtgate/semantics/analyzer.go | 11 ++++++---- go/vt/vtgate/semantics/analyzer_test.go | 2 +- go/vt/vtgate/semantics/info_schema.go | 4 ++++ go/vt/vtgate/semantics/semantic_state.go | 3 ++- 8 files changed, 37 insertions(+), 14 deletions(-) diff --git a/go/test/vschemawrapper/vschema_wrapper.go b/go/test/vschemawrapper/vschema_wrapper.go index 78cf0f8d41c..04cedba581a 100644 --- a/go/test/vschemawrapper/vschema_wrapper.go +++ b/go/test/vschemawrapper/vschema_wrapper.go @@ -40,14 +40,15 @@ import ( var _ plancontext.VSchema = (*VSchemaWrapper)(nil) type VSchemaWrapper struct { - V *vindexes.VSchema - Keyspace *vindexes.Keyspace - TabletType_ topodatapb.TabletType - Dest key.Destination - SysVarEnabled bool - Version plancontext.PlannerVersion - EnableViews bool - TestBuilder func(query string, vschema plancontext.VSchema, keyspace string) (*engine.Plan, error) + V *vindexes.VSchema + Keyspace *vindexes.Keyspace + TabletType_ topodatapb.TabletType + Dest key.Destination + SysVarEnabled bool + ForeignKeyChecksState sqlparser.FkChecksState + Version plancontext.PlannerVersion + EnableViews bool + TestBuilder func(query string, vschema plancontext.VSchema, keyspace string) (*engine.Plan, error) } func (vw *VSchemaWrapper) GetPrepareData(stmtName string) *vtgatepb.PrepareData { @@ -140,6 +141,10 @@ func (vw *VSchemaWrapper) KeyspaceError(keyspace string) error { return nil } +func (vw *VSchemaWrapper) GetForeignKeyChecksState() sqlparser.FkChecksState { + return vw.ForeignKeyChecksState +} + func (vw *VSchemaWrapper) AllKeyspace() ([]*vindexes.Keyspace, error) { if vw.Keyspace == nil { return nil, vterrors.VT13001("keyspace not available") diff --git a/go/vt/schemadiff/semantics.go b/go/vt/schemadiff/semantics.go index da9c6b1e2a9..c2c4580ba28 100644 --- a/go/vt/schemadiff/semantics.go +++ b/go/vt/schemadiff/semantics.go @@ -64,6 +64,10 @@ func (si *declarativeSchemaInformation) KeyspaceError(keyspace string) error { return nil } +func (si *declarativeSchemaInformation) GetForeignKeyChecksState() sqlparser.FkChecksState { + return sqlparser.FkChecksUnspecified +} + // addTable adds a fake table with an empty column list func (si *declarativeSchemaInformation) addTable(tableName string) { tbl := &vindexes.Table{ diff --git a/go/vt/vtgate/planbuilder/plancontext/vschema.go b/go/vt/vtgate/planbuilder/plancontext/vschema.go index 9dde6dee31c..10fbd4c4e80 100644 --- a/go/vt/vtgate/planbuilder/plancontext/vschema.go +++ b/go/vt/vtgate/planbuilder/plancontext/vschema.go @@ -60,6 +60,8 @@ type VSchema interface { // KeyspaceError returns any error in the keyspace vschema. KeyspaceError(keyspace string) error + GetForeignKeyChecksState() sqlparser.FkChecksState + // GetVSchema returns the latest cached vindexes.VSchema GetVSchema() *vindexes.VSchema diff --git a/go/vt/vtgate/semantics/FakeSI.go b/go/vt/vtgate/semantics/FakeSI.go index b7043b42980..f085d9ea92e 100644 --- a/go/vt/vtgate/semantics/FakeSI.go +++ b/go/vt/vtgate/semantics/FakeSI.go @@ -61,6 +61,10 @@ func (s *FakeSI) ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyM return vschemapb.Keyspace_unmanaged, nil } +func (s *FakeSI) GetForeignKeyChecksState() sqlparser.FkChecksState { + return sqlparser.FkChecksUnspecified +} + func (s *FakeSI) KeyspaceError(keyspace string) error { if s.KsError != nil { fkErr, isPresent := s.KsError[keyspace] diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index e524b1a33cf..4ee6797e390 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -77,7 +77,7 @@ func Analyze(statement sqlparser.Statement, currentDb string, si SchemaInformati } // Creation of the semantic table - return analyzer.newSemTable(statement, si.ConnCollation()) + return analyzer.newSemTable(statement, si.ConnCollation(), si.GetForeignKeyChecksState()) } // AnalyzeStrict analyzes the parsed query, and fails the analysis for any possible errors @@ -97,7 +97,7 @@ func AnalyzeStrict(statement sqlparser.Statement, currentDb string, si SchemaInf return st, nil } -func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID) (*SemTable, error) { +func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID, fkChecksState sqlparser.FkChecksState) (*SemTable, error) { var comments *sqlparser.ParsedComments commentedStmt, isCommented := statement.(sqlparser.Commented) if isCommented { @@ -108,7 +108,7 @@ func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID columns[union] = info.exprs } - childFks, parentFks, err := a.getInvolvedForeignKeys(statement) + childFks, parentFks, err := a.getInvolvedForeignKeys(statement, fkChecksState) if err != nil { return nil, err } @@ -317,7 +317,10 @@ func (a *analyzer) noteQuerySignature(node sqlparser.SQLNode) { } // getInvolvedForeignKeys gets the foreign keys that might require taking care off when executing the given statement. -func (a *analyzer) getInvolvedForeignKeys(statement sqlparser.Statement) (map[TableSet][]vindexes.ChildFKInfo, map[TableSet][]vindexes.ParentFKInfo, error) { +func (a *analyzer) getInvolvedForeignKeys(statement sqlparser.Statement, fkChecksState sqlparser.FkChecksState) (map[TableSet][]vindexes.ChildFKInfo, map[TableSet][]vindexes.ParentFKInfo, error) { + if fkChecksState == sqlparser.FkChecksOff { + return nil, nil, nil + } // There are only the DML statements that require any foreign keys handling. switch stmt := statement.(type) { case *sqlparser.Delete: diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index c8251dd36c3..b261a53e9ef 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -2152,7 +2152,7 @@ func TestGetInvolvedForeignKeys(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - childFks, parentFks, err := tt.analyzer.getInvolvedForeignKeys(tt.stmt) + childFks, parentFks, err := tt.analyzer.getInvolvedForeignKeys(tt.stmt, sqlparser.FkChecksOn) if tt.expectedErr != "" { require.EqualError(t, err, tt.expectedErr) return diff --git a/go/vt/vtgate/semantics/info_schema.go b/go/vt/vtgate/semantics/info_schema.go index 838f6276472..d402f2f7ca5 100644 --- a/go/vt/vtgate/semantics/info_schema.go +++ b/go/vt/vtgate/semantics/info_schema.go @@ -1718,6 +1718,10 @@ func (i *infoSchemaWithColumns) ForeignKeyMode(keyspace string) (vschemapb.Keysp return i.inner.ForeignKeyMode(keyspace) } +func (i *infoSchemaWithColumns) GetForeignKeyChecksState() sqlparser.FkChecksState { + return sqlparser.FkChecksUnspecified +} + func (i *infoSchemaWithColumns) KeyspaceError(keyspace string) error { return i.inner.KeyspaceError(keyspace) } diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 94b1302b357..d6dd881919c 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -144,12 +144,13 @@ type ( ColumnName string } - // SchemaInformation is used tp provide table information from Vschema. + // SchemaInformation is used to provide table information from Vschema. SchemaInformation interface { FindTableOrVindex(tablename sqlparser.TableName) (*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error) ConnCollation() collations.ID // ForeignKeyMode returns the foreign_key flag value ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyMode, error) + GetForeignKeyChecksState() sqlparser.FkChecksState KeyspaceError(keyspace string) error } From 59666651adda661366a2f7f29048f88aca1d39bd Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 9 Nov 2023 12:23:15 +0530 Subject: [PATCH 09/25] feat: add tests and fix minor bugs Signed-off-by: Manan Gupta --- go/vt/sqlparser/ast_rewriting.go | 3 +- go/vt/sqlparser/comments.go | 22 +- go/vt/sqlparser/comments_test.go | 4 +- go/vt/vtgate/planbuilder/builder.go | 11 +- go/vt/vtgate/planbuilder/operators/delete.go | 13 +- go/vt/vtgate/planbuilder/operators/update.go | 15 +- go/vt/vtgate/planbuilder/plan_test.go | 15 + .../testdata/foreignkey_cases.json | 89 +- .../testdata/foreignkey_checks_on_cases.json | 1732 +++++++++++++++++ 9 files changed, 1880 insertions(+), 24 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json diff --git a/go/vt/sqlparser/ast_rewriting.go b/go/vt/sqlparser/ast_rewriting.go index 6d0c6eaec85..86017009313 100644 --- a/go/vt/sqlparser/ast_rewriting.go +++ b/go/vt/sqlparser/ast_rewriting.go @@ -191,7 +191,8 @@ func (er *astRewriter) rewriteUp(cursor *Cursor) bool { supportOptimizerHint.SetComments(newComments) } if er.fkChecksState != FkChecksUnspecified { - supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, er.fkChecksState.String()) + newComments := supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, er.fkChecksState.String()) + supportOptimizerHint.SetComments(newComments) } } diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index a840b05308d..c5f3c4505c5 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -307,17 +307,19 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { } // SetMySQLSetVarValue updates or sets the value of the given variable as part of a /*+ SET_VAR() */ MySQL optimizer hint. -func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) { +func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComments Comments) { if c == nil { - c = &ParsedComments{ - comments: Comments{}, - } + newComments = append(newComments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) + return } - for idx, commentStr := range c.comments { - if commentStr[0:3] != queryOptimizerPrefix { + seenFirstOhComment := false + for _, commentStr := range c.comments { + if seenFirstOhComment || commentStr[0:3] != queryOptimizerPrefix { + newComments = append(newComments, commentStr) continue } + seenFirstOhComment = true finalComment := "/*+" keyPresent := false pos := 4 @@ -348,10 +350,12 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) { } finalComment += " */" - c.comments[idx] = finalComment - return + newComments = append(newComments, finalComment) + } + if !seenFirstOhComment { + newComments = append(newComments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) } - c.comments = append(c.comments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) + return newComments } func getOptimizerHint(initialPos int, commentStr string) (pos int, ohNameStart int, ohNameEnd int, ohContentStart int, ohContentEnd int) { diff --git a/go/vt/sqlparser/comments_test.go b/go/vt/sqlparser/comments_test.go index e5bcda1d4ba..6312acb5994 100644 --- a/go/vt/sqlparser/comments_test.go +++ b/go/vt/sqlparser/comments_test.go @@ -664,8 +664,8 @@ func TestSetMySQLSetVarValue(t *testing.T) { c := &ParsedComments{ comments: tt.comments, } - c.SetMySQLSetVarValue(tt.key, tt.value) - require.EqualValues(t, tt.commentsWanted, c.comments) + newComments := c.SetMySQLSetVarValue(tt.key, tt.value) + require.EqualValues(t, tt.commentsWanted, newComments) }) } } diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 0b778914739..fca5464b4ab 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -22,6 +22,7 @@ import ( "sort" "vitess.io/vitess/go/sqltypes" + "vitess.io/vitess/go/test/vschemawrapper" "vitess.io/vitess/go/vt/key" "vitess.io/vitess/go/vt/log" querypb "vitess.io/vitess/go/vt/proto/query" @@ -74,7 +75,15 @@ func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*e if err != nil { return nil, err } - result, err := sqlparser.RewriteAST(stmt, keyspace, sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, vschema) + // Store the foreign key mode like we do for vcursor. + vw, isVw := vschema.(*vschemawrapper.VSchemaWrapper) + if isVw { + fkState := sqlparser.ForeignKeyChecksState(stmt) + if fkState != sqlparser.FkChecksUnspecified { + vw.ForeignKeyChecksState = fkState + } + } + result, err := sqlparser.RewriteAST(stmt, keyspace, sqlparser.SQLSelectLimitUnset, "", nil, vschema.GetForeignKeyChecksState(), vschema) if err != nil { return nil, err } diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index f02444671c1..bf360ffc123 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -20,6 +20,7 @@ import ( "fmt" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/sysvars" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" @@ -204,7 +205,15 @@ func createFkCascadeOpForDelete(ctx *plancontext.PlanningContext, parentOp ops.O func createFkChildForDelete(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, cols []int) (*FkChild, error) { bvName := ctx.ReservedVars.ReserveVariable(foreignKeyConstraintValues) - + // We only reach this code if foreign key checks are either unspecified or on. + // If foreign key checks are explicity turned on, then we should add the set_var parsed comment too + // since underlying MySQL might have foreign_key_checks as off. + // We run with foreign key checks on because the delete might still fail on MySQL due to a child table + // with RESTRICT constraints. + var parsedComments *sqlparser.ParsedComments + if ctx.VSchema.GetForeignKeyChecksState() == sqlparser.FkChecksOn { + parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, sqlparser.FkChecksOn.String()).Parsed() + } var childStmt sqlparser.Statement switch fk.OnDelete { case sqlparser.Cascade: @@ -216,6 +225,7 @@ func createFkChildForDelete(ctx *plancontext.PlanningContext, fk vindexes.ChildF } compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) childStmt = &sqlparser.Delete{ + Comments: parsedComments, TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, } @@ -234,6 +244,7 @@ func createFkChildForDelete(ctx *plancontext.PlanningContext, fk vindexes.ChildF compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, valTuple, sqlparser.NewListArg(bvName), nil) childStmt = &sqlparser.Update{ Exprs: updExprs, + Comments: parsedComments, TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: compExpr}, } diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 52dae6bd9b5..1a1d77717eb 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -23,6 +23,7 @@ import ( "strings" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/sysvars" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" @@ -352,9 +353,7 @@ func buildChildUpdOpForCascade(ctx *plancontext.PlanningContext, fk vindexes.Chi // Because we could be updating the child to a non-null value, // We have to run with foreign key checks OFF because the parent isn't guaranteed to have // the data being updated to. - parsedComments := sqlparser.Comments{ - "/*+ SET_VAR(foreign_key_checks=OFF) */", - }.Parsed() + parsedComments := (&sqlparser.ParsedComments{}).SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, sqlparser.FkChecksOff.String()).Parsed() childUpdStmt := &sqlparser.Update{ Comments: parsedComments, Exprs: childUpdateExprs, @@ -399,8 +398,18 @@ func buildChildUpdOpForSetNull(ctx *plancontext.PlanningContext, fk vindexes.Chi Right: compExpr, } } + // We only reach this code if foreign key checks are either unspecified or on. + // If foreign key checks are explicity turned on, then we should add the set_var parsed comment too + // since underlying MySQL might have foreign_key_checks as off. + // We run with foreign key checks on because the update might still fail on MySQL due to a child table + // with RESTRICT constraints. + var parsedComments *sqlparser.ParsedComments + if ctx.VSchema.GetForeignKeyChecksState() == sqlparser.FkChecksOn { + parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, sqlparser.FkChecksOn.String()).Parsed() + } childUpdStmt := &sqlparser.Update{ Exprs: childUpdateExprs, + Comments: parsedComments, TableExprs: []sqlparser.TableExpr{sqlparser.NewAliasedTableExpr(fk.Table.GetTableName(), "")}, Where: &sqlparser.Where{Type: sqlparser.WhereClause, Expr: childWhereExpr}, } diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 472648828ef..44843d655cc 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -113,6 +113,21 @@ func TestForeignKeyPlanning(t *testing.T) { testFile(t, "foreignkey_cases.json", testOutputTempDir, vschemaWrapper, false) } +// TestForeignKeyChecksOn tests the planning when the session variable for foreign_key_checks is set to ON. +func TestForeignKeyChecksOn(t *testing.T) { + vschema := loadSchema(t, "vschemas/schema.json", true) + setFks(t, vschema) + vschemaWrapper := &vschemawrapper.VSchemaWrapper{ + V: vschema, + TestBuilder: TestBuilder, + ForeignKeyChecksState: sqlparser.FkChecksOn, + } + + testOutputTempDir := makeTestOutput(t) + + testFile(t, "foreignkey_checks_on_cases.json", testOutputTempDir, vschemaWrapper, false) +} + func setFks(t *testing.T, vschema *vindexes.VSchema) { if vschema.Keyspaces["sharded_fk_allow"] != nil { // FK from multicol_tbl2 referencing multicol_tbl1 that is shard scoped. diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 065691d2356..45eebef31eb 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -705,7 +705,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -921,7 +921,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = 2 where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 2 where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -1144,7 +1144,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl8 set col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", "Table": "u_tbl8" } ] @@ -1232,7 +1232,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1321,7 +1321,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1441,7 +1441,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -1535,7 +1535,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -1653,5 +1653,80 @@ "sharded_fk_allow.tbl5" ] } + }, + { + "comment": "Delete with foreign key checks off", + "query": "delete /*+ SET_VAR(foreign_key_checks=off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "plan": { + "QueryType": "DELETE", + "Original": "delete /*+ SET_VAR(foreign_key_checks=off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Instructions": { + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=Off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Table": "multicol_tbl1", + "Values": [ + "1", + "2", + "3" + ], + "Vindex": "multicolIdx" + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl1" + ] + } + }, + { + "comment": "Update with foreign key checks off", + "query": "update /*+ SET_VAR(foreign_key_checks=0) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "plan": { + "QueryType": "UPDATE", + "Original": "update /*+ SET_VAR(foreign_key_checks=0) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Instructions": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Table": "u_multicol_tbl1" + }, + "TablesUsed": [ + "unsharded_fk_allow.u_multicol_tbl1" + ] + } + }, + { + "comment": "Insert with cross shard foreign keys and foreign key checks off", + "query": "insert /*+ SET_VAR(foreign_key_checks=0) */ into tbl3 (col3, coly) values (1, 3)", + "plan": { + "QueryType": "INSERT", + "Original": "insert /*+ SET_VAR(foreign_key_checks=0) */ into tbl3 (col3, coly) values (1, 3)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=Off) */ into tbl3(col3, coly) values (:_col3_0, 3)", + "TableName": "tbl3", + "VindexValues": { + "hash_vin": "1" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl3" + ] + } } ] diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json new file mode 100644 index 00000000000..943c5005c35 --- /dev/null +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json @@ -0,0 +1,1732 @@ +[ + { + "comment": "Insertion in a table with cross-shard foreign keys disallowed", + "query": "insert into tbl3 (col3, coly) values (1, 3)", + "plan": "VT12002: unsupported: cross-shard foreign keys" + }, + { + "comment": "Insertion in a table with shard-scoped foreign keys is allowed", + "query": "insert into tbl2 (col2, coly) values (1, 3)", + "plan": { + "QueryType": "INSERT", + "Original": "insert into tbl2 (col2, coly) values (1, 3)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=On) */ into tbl2(col2, coly) values (:_col2_0, 3)", + "TableName": "tbl2", + "VindexValues": { + "hash_vin": "1" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl2" + ] + } + }, + { + "comment": "Insertion in a table with shard-scoped multiple column foreign key is allowed", + "query": "insert into multicol_tbl2 (cola, colb, colc) values (1, 2, 3)", + "plan": { + "QueryType": "INSERT", + "Original": "insert into multicol_tbl2 (cola, colb, colc) values (1, 2, 3)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=On) */ into multicol_tbl2(cola, colb, colc) values (:_cola_0, :_colb_0, :_colc_0)", + "TableName": "multicol_tbl2", + "VindexValues": { + "multicolIdx": "1, 2, 3" + } + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl2" + ] + } + }, + { + "comment": "Delete in a table with cross-shard foreign keys disallowed", + "query": "delete from tbl1", + "plan": "VT12002: unsupported: cross-shard foreign keys" + }, + { + "comment": "Delete in a table with not all column shard-scoped foreign keys - disallowed", + "query": "delete from tbl7", + "plan": "VT12002: unsupported: cross-shard foreign keys" + }, + { + "comment": "Delete in a table with shard-scoped multiple column foreign key with cascade", + "query": "delete from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "plan": { + "QueryType": "DELETE", + "Original": "delete from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select colb, cola, y, colc, x from multicol_tbl1 where 1 != 1", + "Query": "select colb, cola, y, colc, x from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3 for update", + "Table": "multicol_tbl1", + "Values": [ + "1", + "2", + "3" + ], + "Vindex": "multicolIdx" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0, + 1, + 2, + 3, + 4 + ], + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from multicol_tbl2 where (colb, cola, x, colc, y) in ::fkc_vals", + "Table": "multicol_tbl2" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Table": "multicol_tbl1", + "Values": [ + "1", + "2", + "3" + ], + "Vindex": "multicolIdx" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl1", + "sharded_fk_allow.multicol_tbl2" + ] + } + }, + { + "comment": "Delete in a table with shard-scoped foreign keys with cascade", + "query": "delete from tbl5", + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl5", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select col5, t5col5 from tbl5 where 1 != 1", + "Query": "select col5, t5col5 from tbl5 for update", + "Table": "tbl5" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (col4) in ::fkc_vals", + "Table": "tbl4" + }, + { + "InputName": "CascadeChild-2", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 1 + ], + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (t4col4) in ::fkc_vals1", + "Table": "tbl4" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl5", + "Table": "tbl5" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl5" + ] + } + }, + { + "comment": "Delete in a table with shard-scoped foreign keys with SET NULL", + "query": "delete from tbl8 where col8 = 1", + "plan": "VT12001: unsupported: you cannot UPDATE primary vindex columns; invalid update on vindex: hash_vin" + }, + { + "comment": "Delete in a table with unsharded foreign key with SET NULL", + "query": "delete from u_tbl9 where col9 = 5", + "plan": { + "QueryType": "DELETE", + "Original": "delete from u_tbl9 where col9 = 5", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col9 from u_tbl9 where 1 != 1", + "Query": "select col9 from u_tbl9 where col9 = 5 for update", + "Table": "u_tbl9" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals", + "Table": "u_tbl8" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from u_tbl9 where col9 = 5", + "Table": "u_tbl9" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl8", + "unsharded_fk_allow.u_tbl9" + ] + } + }, + { + "comment": "update in unsharded table with restrict", + "query": "update u_tbl5 set col5 = 'foo' where id = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl5 set col5 = 'foo' where id = 1", + "Instructions": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl5 set col5 = 'foo' where id = 1", + "Table": "u_tbl5" + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl5" + ] + } + }, + { + "comment": "update in unsharded table with cascade", + "query": "update u_tbl2 set col2 = 'bar' where id = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set col2 = 'bar' where id = 1", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2 from u_tbl2 where 1 != 1", + "Query": "select col2 from u_tbl2 where id = 1 for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in (('bar'))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl2 set col2 = 'bar' where id = 1", + "Table": "u_tbl2" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } + }, + { + "comment": "update in unsharded table with cascade - on non-referenced column", + "query": "update u_tbl2 set col_no_ref = 'baz' where id = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set col_no_ref = 'baz' where id = 1", + "Instructions": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl2 set col_no_ref = 'baz' where id = 1", + "Table": "u_tbl2" + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2" + ] + } + }, + { + "comment": "Update in a table with cross-shard foreign keys disallowed", + "query": "update tbl1 set t1col1 = 'foo' where col1 = 1", + "plan": "VT12002: unsupported: cross-shard foreign keys" + }, + { + "comment": "Update in a table with cross-shard foreign keys, column not in update expression - allowed", + "query": "update tbl1 set not_ref_col = 'foo' where id = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl1 set not_ref_col = 'foo' where id = 1", + "Instructions": { + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl1 set not_ref_col = 'foo' where id = 1", + "Table": "tbl1" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl1" + ] + } + }, + { + "comment": "Update in a table with column modified not shard-scoped foreign key whereas other column referencing same table is - disallowed", + "query": "update tbl7 set t7col7 = 'foo', t7col72 = 42", + "plan": "VT12002: unsupported: cross-shard foreign keys" + }, + { + "comment": "Update in a table with shard-scoped foreign keys with cascade", + "query": "update tbl5 set t5col5 = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl5 set t5col5 = 'foo'", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select t5col5 from tbl5 where 1 != 1", + "Query": "select t5col5 from tbl5 for update", + "Table": "tbl5" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (tbl4.t4col4) not in (('foo'))", + "Table": "tbl4" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl5 set t5col5 = 'foo'", + "Table": "tbl5" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl5" + ] + } + }, + { + "comment": "Insertion in a table with 2 foreign keys constraint with same table on different columns - both are not shard scoped - disallowed", + "query": "insert into tbl6 (col6, t6col6) values (100, 'foo')", + "plan": "VT12002: unsupported: cross-shard foreign keys" + }, + { + "comment": "Update a table with parent and child foreign keys - shard scoped", + "query": "update tbl2 set col = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl2 set col = 'foo'", + "Instructions": { + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl2 set col = 'foo'", + "Table": "tbl2" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl2" + ] + } + }, + { + "comment": "update table with column's parent foreign key cross shard", + "query": "update tbl10 set col = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl10 set col = 'foo'", + "Instructions": { + "OperatorType": "FKVerify", + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Limit", + "Count": "1", + "Inputs": [ + { + "OperatorType": "Projection", + "Expressions": [ + "1 as 1" + ], + "Inputs": [ + { + "OperatorType": "Filter", + "Predicate": "tbl3.col is null", + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinColumnIndexes": "R:0,R:0", + "TableName": "tbl10_tbl3", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select 1 from tbl10 where 1 != 1", + "Query": "select 1 from tbl10 lock in share mode", + "Table": "tbl10" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select tbl3.col from tbl3 where 1 != 1", + "Query": "select tbl3.col from tbl3 where tbl3.col = 'foo' lock in share mode", + "Table": "tbl3" + } + ] + } + ] + } + ] + } + ] + }, + { + "InputName": "PostVerify", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl10 set col = 'foo'", + "Table": "tbl10" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl10", + "sharded_fk_allow.tbl3" + ] + } + }, + { + "comment": "delete table with shard scoped foreign key set default - disallowed", + "query": "delete from tbl20 where col = 'bar'", + "plan": "VT09016: Cannot delete or update a parent row: a foreign key constraint fails" + }, + { + "comment": "Delete table with cross-shard foreign key with set null - should be eventually allowed", + "query": "delete from tbl9 where col9 = 34", + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl9 where col9 = 34", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select col9 from tbl9 where 1 != 1", + "Query": "select col9 from tbl9 where col9 = 34 for update", + "Table": "tbl9", + "Values": [ + "34" + ], + "Vindex": "hash_vin" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set col_ref = null where (col_ref) in ::fkc_vals", + "Table": "tbl4" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl9 where col9 = 34", + "Table": "tbl9", + "Values": [ + "34" + ], + "Vindex": "hash_vin" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl9" + ] + } + }, + { + "comment": "update table with same column having reference to different tables, one with on update cascade other with on update set null - child table have further reference", + "query": "update u_tbl1 set col1 = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl1 set col1 = 'foo'", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col1, col1 from u_tbl1 where 1 != 1", + "Query": "select col1, col1 from u_tbl1 for update", + "Table": "u_tbl1" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FkCascade", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2 from u_tbl2 where 1 != 1", + "Query": "select col2 from u_tbl2 where (col2) in ::fkc_vals for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in (('foo'))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", + "Table": "u_tbl2" + } + ] + }, + { + "InputName": "CascadeChild-2", + "OperatorType": "FkCascade", + "BvName": "fkc_vals2", + "Cols": [ + 1 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col9 from u_tbl9 where 1 != 1", + "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in (('foo')) for update", + "Table": "u_tbl9" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals3", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", + "Table": "u_tbl8" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in (('foo'))", + "Table": "u_tbl9" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl1 set col1 = 'foo'", + "Table": "u_tbl1" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl1", + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3", + "unsharded_fk_allow.u_tbl8", + "unsharded_fk_allow.u_tbl9" + ] + } + }, + { + "comment": "update in a table with limit - disallowed", + "query": "update u_tbl2 set col2 = 'bar' limit 2", + "plan": "VT12001: unsupported: update with limit with foreign key constraints" + }, + { + "comment": "update in a table with non-literal value - set null fail due to child update where condition", + "query": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1", + "plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints" + }, + { + "comment": "update in a table with non-literal value - with cascade fail as the cascade value is not known", + "query": "update u_tbl1 set m = 2, col1 = x + 'bar' where id = 1", + "plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints" + }, + { + "comment": "update in a table with set null, non-literal value on non-foreign key column - allowed", + "query": "update u_tbl2 set m = col1 + 'bar', col2 = 2 where id = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set m = col1 + 'bar', col2 = 2 where id = 1", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2 from u_tbl2 where 1 != 1", + "Query": "select col2 from u_tbl2 where id = 1 for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in ((2))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl2 set m = col1 + 'bar', col2 = 2 where id = 1", + "Table": "u_tbl2" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } + }, + { + "comment": "update in a table with cascade, non-literal value on non-foreign key column - allowed", + "query": "update u_tbl1 set m = x + 'bar', col1 = 2 where id = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl1 set m = x + 'bar', col1 = 2 where id = 1", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col1, col1 from u_tbl1 where 1 != 1", + "Query": "select col1, col1 from u_tbl1 where id = 1 for update", + "Table": "u_tbl1" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FkCascade", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2 from u_tbl2 where 1 != 1", + "Query": "select col2 from u_tbl2 where (col2) in ::fkc_vals for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in ((2))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 2 where (col2) in ::fkc_vals", + "Table": "u_tbl2" + } + ] + }, + { + "InputName": "CascadeChild-2", + "OperatorType": "FkCascade", + "BvName": "fkc_vals2", + "Cols": [ + 1 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col9 from u_tbl9 where 1 != 1", + "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in ((2)) for update", + "Table": "u_tbl9" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals3", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", + "Table": "u_tbl8" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in ((2))", + "Table": "u_tbl9" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl1 set m = x + 'bar', col1 = 2 where id = 1", + "Table": "u_tbl1" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl1", + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3", + "unsharded_fk_allow.u_tbl8", + "unsharded_fk_allow.u_tbl9" + ] + } + }, + { + "comment": "update in a table with a child table having SET DEFAULT constraint - disallowed", + "query": "update tbl20 set col2 = 'bar'", + "plan": "VT09016: Cannot delete or update a parent row: a foreign key constraint fails" + }, + { + "comment": "delete in a table with limit - disallowed", + "query": "delete from u_tbl2 limit 2", + "plan": "VT12001: unsupported: foreign keys management at vitess with limit" + }, + { + "comment": "update with fk on cross-shard with a where condition on non-literal value - disallowed", + "query": "update tbl3 set coly = colx + 10 where coly = 10", + "plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints" + }, + { + "comment": "update with fk on cross-shard with a where condition", + "query": "update tbl3 set coly = 20 where coly = 10", + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl3 set coly = 20 where coly = 10", + "Instructions": { + "OperatorType": "FKVerify", + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Limit", + "Count": "1", + "Inputs": [ + { + "OperatorType": "Projection", + "Expressions": [ + "1 as 1" + ], + "Inputs": [ + { + "OperatorType": "Filter", + "Predicate": "tbl1.t1col1 is null", + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinColumnIndexes": "R:0,R:0", + "TableName": "tbl3_tbl1", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select 1 from tbl3 where 1 != 1", + "Query": "select 1 from tbl3 where tbl3.coly = 10 lock in share mode", + "Table": "tbl3" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select tbl1.t1col1 from tbl1 where 1 != 1", + "Query": "select tbl1.t1col1 from tbl1 where tbl1.t1col1 = 20 lock in share mode", + "Table": "tbl1" + } + ] + } + ] + } + ] + } + ] + }, + { + "InputName": "PostVerify", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl3 set coly = 20 where tbl3.coly = 10", + "Table": "tbl3" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl1", + "sharded_fk_allow.tbl3" + ] + } + }, + { + "comment": "Update in a table with shard-scoped foreign keys with cascade that requires a validation of a different parent foreign key", + "query": "update u_tbl6 set col6 = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl6 set col6 = 'foo'", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col6 from u_tbl6 where 1 != 1", + "Query": "select col6 from u_tbl6 for update", + "Table": "u_tbl6" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FKVerify", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where 1 != 1", + "Query": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where (u_tbl8.col8) in ::fkc_vals and u_tbl9.col9 is null limit 1 lock in share mode", + "Table": "u_tbl8, u_tbl9" + }, + { + "InputName": "PostVerify", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", + "Table": "u_tbl8" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl6 set col6 = 'foo'", + "Table": "u_tbl6" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl6", + "unsharded_fk_allow.u_tbl8", + "unsharded_fk_allow.u_tbl9" + ] + } + }, + { + "comment": "Update that cascades and requires parent fk and restrict child fk verification", + "query": "update u_tbl7 set col7 = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl7 set col7 = 'foo'", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col7 from u_tbl7 where 1 != 1", + "Query": "select col7 from u_tbl7 for update", + "Table": "u_tbl7" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FKVerify", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where 1 != 1", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", + "Table": "u_tbl3, u_tbl4" + }, + { + "InputName": "VerifyChild-2", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl4, u_tbl9 where 1 != 1", + "Query": "select 1 from u_tbl4, u_tbl9 where (u_tbl4.col4) in ::fkc_vals and (u_tbl9.col9) not in (('foo')) and u_tbl4.col4 = u_tbl9.col9 limit 1 lock in share mode", + "Table": "u_tbl4, u_tbl9" + }, + { + "InputName": "PostVerify", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", + "Table": "u_tbl4" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl7 set col7 = 'foo'", + "Table": "u_tbl7" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl3", + "unsharded_fk_allow.u_tbl4", + "unsharded_fk_allow.u_tbl7", + "unsharded_fk_allow.u_tbl9" + ] + } + }, + { + "comment": "Update that cascades and requires parent fk and restrict child fk verification - bindVariable", + "query": "update u_tbl7 set col7 = :v1", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl7 set col7 = :v1", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col7 from u_tbl7 where 1 != 1", + "Query": "select col7 from u_tbl7 for update", + "Table": "u_tbl7" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FKVerify", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where 1 != 1", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", + "Table": "u_tbl3, u_tbl4" + }, + { + "InputName": "VerifyChild-2", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl4, u_tbl9 where 1 != 1", + "Query": "select 1 from u_tbl4, u_tbl9 where (u_tbl4.col4) in ::fkc_vals and (:v1 is null or (u_tbl9.col9) not in ((:v1))) and u_tbl4.col4 = u_tbl9.col9 limit 1 lock in share mode", + "Table": "u_tbl4, u_tbl9" + }, + { + "InputName": "PostVerify", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", + "Table": "u_tbl4" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl7 set col7 = :v1", + "Table": "u_tbl7" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl3", + "unsharded_fk_allow.u_tbl4", + "unsharded_fk_allow.u_tbl7", + "unsharded_fk_allow.u_tbl9" + ] + } + }, + { + "comment": "Insert with on duplicate key update - foreign keys disallowed", + "query": "insert into u_tbl1 (id, col1) values (1, 3) on duplicate key update col1 = 5", + "plan": "VT12001: unsupported: ON DUPLICATE KEY UPDATE with foreign keys" + }, + { + "comment": "Insert with on duplicate key update - foreign keys not on update column - allowed", + "query": "insert into u_tbl1 (id, col1, foo) values (1, 3, 'bar') on duplicate key update foo = 'baz'", + "plan": { + "QueryType": "INSERT", + "Original": "insert into u_tbl1 (id, col1, foo) values (1, 3, 'bar') on duplicate key update foo = 'baz'", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=On) */ into u_tbl1(id, col1, foo) values (1, 3, 'bar') on duplicate key update foo = 'baz'", + "TableName": "u_tbl1" + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl1" + ] + } + }, + { + "comment": "Insert with unsharded table having fk reference in sharded table", + "query": "insert into u_tbl (id, col) values (1, 2)", + "plan": "VT12002: unsupported: cross-shard foreign keys" + }, + { + "comment": "replace with fk reference unsupported", + "query": "replace into u_tbl1 (id, col1) values (1, 2)", + "plan": "VT12001: unsupported: REPLACE INTO with foreign keys" + }, + { + "comment": "update on a multicol foreign key that set nulls and then cascades", + "query": "update u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select cola, colb from u_multicol_tbl1 where 1 != 1", + "Query": "select cola, colb from u_multicol_tbl1 where id = 3 for update", + "Table": "u_multicol_tbl1" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FkCascade", + "BvName": "fkc_vals", + "Cols": [ + 0, + 1 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select cola, colb from u_multicol_tbl2 where 1 != 1", + "Query": "select cola, colb from u_multicol_tbl2 where (cola, colb) in ::fkc_vals and (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((1, 2)) for update", + "Table": "u_multicol_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0, + 1 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Table": "u_multicol_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((1, 2))", + "Table": "u_multicol_tbl2" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Table": "u_multicol_tbl1" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_multicol_tbl1", + "unsharded_fk_allow.u_multicol_tbl2", + "unsharded_fk_allow.u_multicol_tbl3" + ] + } + }, + { + "comment": "update on a multicol foreign key that set nulls and then cascades - bindVariables", + "query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl1 set cola = :v1, colb = :v2 where id = :v3", + "plan": { + "QueryType": "UPDATE", + "Original": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl1 set cola = :v1, colb = :v2 where id = :v3", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select cola, colb from u_multicol_tbl1 where 1 != 1", + "Query": "select cola, colb from u_multicol_tbl1 where id = :v3 for update", + "Table": "u_multicol_tbl1" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FkCascade", + "BvName": "fkc_vals", + "Cols": [ + 0, + 1 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select cola, colb from u_multicol_tbl2 where 1 != 1", + "Query": "select cola, colb from u_multicol_tbl2 where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((:v1, :v2)))) for update", + "Table": "u_multicol_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0, + 1 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Table": "u_multicol_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((:v1, :v2))))", + "Table": "u_multicol_tbl2" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl1 set cola = :v1, colb = :v2 where id = :v3", + "Table": "u_multicol_tbl1" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_multicol_tbl1", + "unsharded_fk_allow.u_multicol_tbl2", + "unsharded_fk_allow.u_multicol_tbl3" + ] + } + }, + { + "comment": "Cascaded delete run from prepared statement", + "query": "execute prep_delete using @foo", + "plan": { + "QueryType": "EXECUTE", + "Original": "execute prep_delete using @foo", + "Instructions": { + "OperatorType": "EXECUTE", + "Parameters": [ + "foo" + ], + "Inputs": [ + { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select col5, t5col5 from tbl5 where 1 != 1", + "Query": "select col5, t5col5 from tbl5 where id = :v1 for update", + "Table": "tbl5" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (col4) in ::fkc_vals", + "Table": "tbl4" + }, + { + "InputName": "CascadeChild-2", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 1 + ], + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (t4col4) in ::fkc_vals1", + "Table": "tbl4" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl5 where id = :v1", + "Table": "tbl5" + } + ] + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl5" + ] + } + }, + { + "comment": "Delete with foreign key checks off", + "query": "delete /*+ SET_VAR(foreign_key_checks=off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "plan": { + "QueryType": "DELETE", + "Original": "delete /*+ SET_VAR(foreign_key_checks=off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Instructions": { + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=Off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Table": "multicol_tbl1", + "Values": [ + "1", + "2", + "3" + ], + "Vindex": "multicolIdx" + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl1" + ] + } + }, + { + "comment": "Update with foreign key checks off", + "query": "update /*+ SET_VAR(foreign_key_checks=0) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "plan": { + "QueryType": "UPDATE", + "Original": "update /*+ SET_VAR(foreign_key_checks=0) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Instructions": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Table": "u_multicol_tbl1" + }, + "TablesUsed": [ + "unsharded_fk_allow.u_multicol_tbl1" + ] + } + }, + { + "comment": "Insert with cross shard foreign keys and foreign key checks off", + "query": "insert /*+ SET_VAR(foreign_key_checks=0) */ into tbl3 (col3, coly) values (1, 3)", + "plan": { + "QueryType": "INSERT", + "Original": "insert /*+ SET_VAR(foreign_key_checks=0) */ into tbl3 (col3, coly) values (1, 3)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=Off) */ into tbl3(col3, coly) values (:_col3_0, 3)", + "TableName": "tbl3", + "VindexValues": { + "hash_vin": "1" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl3" + ] + } + } +] From 48c169ec9afe0e5544efdd53b7bac56900e12074 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 9 Nov 2023 12:36:25 +0530 Subject: [PATCH 10/25] test: add tests for foreign key turned off too Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/builder.go | 5 + go/vt/vtgate/planbuilder/plan_test.go | 15 + .../testdata/foreignkey_checks_off_cases.json | 491 ++++++++++++++++++ 3 files changed, 511 insertions(+) create mode 100644 go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index fca5464b4ab..110b805e3c2 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -80,7 +80,12 @@ func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*e if isVw { fkState := sqlparser.ForeignKeyChecksState(stmt) if fkState != sqlparser.FkChecksUnspecified { + // Restore the old volue of ForeignKeyChecksState to not interfere with the next test cases. + oldVal := vw.ForeignKeyChecksState vw.ForeignKeyChecksState = fkState + defer func() { + vw.ForeignKeyChecksState = oldVal + }() } } result, err := sqlparser.RewriteAST(stmt, keyspace, sqlparser.SQLSelectLimitUnset, "", nil, vschema.GetForeignKeyChecksState(), vschema) diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 44843d655cc..9b06fee7b94 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -128,6 +128,21 @@ func TestForeignKeyChecksOn(t *testing.T) { testFile(t, "foreignkey_checks_on_cases.json", testOutputTempDir, vschemaWrapper, false) } +// TestForeignKeyChecksOff tests the planning when the session variable for foreign_key_checks is set to OFF. +func TestForeignKeyChecksOff(t *testing.T) { + vschema := loadSchema(t, "vschemas/schema.json", true) + setFks(t, vschema) + vschemaWrapper := &vschemawrapper.VSchemaWrapper{ + V: vschema, + TestBuilder: TestBuilder, + ForeignKeyChecksState: sqlparser.FkChecksOff, + } + + testOutputTempDir := makeTestOutput(t) + + testFile(t, "foreignkey_checks_off_cases.json", testOutputTempDir, vschemaWrapper, false) +} + func setFks(t *testing.T, vschema *vindexes.VSchema) { if vschema.Keyspaces["sharded_fk_allow"] != nil { // FK from multicol_tbl2 referencing multicol_tbl1 that is shard scoped. diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json new file mode 100644 index 00000000000..6674ae6a346 --- /dev/null +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json @@ -0,0 +1,491 @@ +[ + { + "comment": "Insertion in a table with cross-shard foreign keys works with foreign_key_checks off", + "query": "insert into tbl3 (col3, coly) values (1, 3)", + "plan": { + "QueryType": "INSERT", + "Original": "insert into tbl3 (col3, coly) values (1, 3)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=Off) */ into tbl3(col3, coly) values (:_col3_0, 3)", + "TableName": "tbl3", + "VindexValues": { + "hash_vin": "1" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl3" + ] + } + }, + { + "comment": "Insertion in a table with shard-scoped multiple column foreign key is allowed", + "query": "insert into multicol_tbl2 (cola, colb, colc) values (1, 2, 3)", + "plan": { + "QueryType": "INSERT", + "Original": "insert into multicol_tbl2 (cola, colb, colc) values (1, 2, 3)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=Off) */ into multicol_tbl2(cola, colb, colc) values (:_cola_0, :_colb_0, :_colc_0)", + "TableName": "multicol_tbl2", + "VindexValues": { + "multicolIdx": "1, 2, 3" + } + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl2" + ] + } + }, + { + "comment": "Delete in a table with cross-shard foreign key works with foreign_key_checks off ", + "query": "delete from tbl1", + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl1", + "Instructions": { + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=Off) */ from tbl1", + "Table": "tbl1" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl1" + ] + } + }, + { + "comment": "Delete in a table with not all column shard-scoped foreign keys works with foreign_key_checks off", + "query": "delete from tbl7", + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl7", + "Instructions": { + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=Off) */ from tbl7", + "Table": "tbl7" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl7" + ] + } + }, + { + "comment": "Delete in a table with shard-scoped multiple column foreign key with cascade with foreign key checks on", + "query": "delete /*+ SET_VAR(foreign_key_checks=1) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "plan": { + "QueryType": "DELETE", + "Original": "delete /*+ SET_VAR(foreign_key_checks=1) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select colb, cola, y, colc, x from multicol_tbl1 where 1 != 1", + "Query": "select colb, cola, y, colc, x from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3 for update", + "Table": "multicol_tbl1", + "Values": [ + "1", + "2", + "3" + ], + "Vindex": "multicolIdx" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Delete", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0, + 1, + 2, + 3, + 4 + ], + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from multicol_tbl2 where (colb, cola, x, colc, y) in ::fkc_vals", + "Table": "multicol_tbl2" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Table": "multicol_tbl1", + "Values": [ + "1", + "2", + "3" + ], + "Vindex": "multicolIdx" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl1", + "sharded_fk_allow.multicol_tbl2" + ] + } + }, + { + "comment": "Delete in a table with shard-scoped foreign keys with SET NULL", + "query": "delete from tbl8 where col8 = 1", + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl8 where col8 = 1", + "Instructions": { + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=Off) */ from tbl8 where col8 = 1", + "Table": "tbl8", + "Values": [ + "1" + ], + "Vindex": "hash_vin" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl8" + ] + } + }, + { + "comment": "Update in a table with cross-shard foreign keys works with foreign_key_checks off", + "query": "update tbl1 set t1col1 = 'foo' where col1 = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl1 set t1col1 = 'foo' where col1 = 1", + "Instructions": { + "OperatorType": "Update", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl1 set t1col1 = 'foo' where col1 = 1", + "Table": "tbl1", + "Values": [ + "1" + ], + "Vindex": "hash_vin" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl1" + ] + } + }, + { + "comment": "Update in a table with column modified not shard-scoped foreign key whereas other column referencing same table is works with foreign_key_checks off", + "query": "update tbl7 set t7col7 = 'foo', t7col72 = 42", + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl7 set t7col7 = 'foo', t7col72 = 42", + "Instructions": { + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl7 set t7col7 = 'foo', t7col72 = 42", + "Table": "tbl7" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl7" + ] + } + }, + { + "comment": "Update in a table with shard-scoped foreign keys with cascade", + "query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl5 set t5col5 = 'foo'", + "plan": { + "QueryType": "UPDATE", + "Original": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl5 set t5col5 = 'foo'", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select t5col5 from tbl5 where 1 != 1", + "Query": "select t5col5 from tbl5 for update", + "Table": "tbl5" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (tbl4.t4col4) not in (('foo'))", + "Table": "tbl4" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl5 set t5col5 = 'foo'", + "Table": "tbl5" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl5" + ] + } + }, + { + "comment": "Insertion in a table with 2 foreign keys constraint with same table on different columns - both are not shard scoped - works with foreign_key_checks off", + "query": "insert into tbl6 (col6, t6col6) values (100, 'foo')", + "plan": { + "QueryType": "INSERT", + "Original": "insert into tbl6 (col6, t6col6) values (100, 'foo')", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=Off) */ into tbl6(col6, t6col6) values (:_col6_0, 'foo')", + "TableName": "tbl6", + "VindexValues": { + "hash_vin": "100" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl6" + ] + } + }, + { + "comment": "delete table with shard scoped foreign key set default works with foreign_key_checks off", + "query": "delete from tbl20 where col = 'bar'", + "plan": { + "QueryType": "DELETE", + "Original": "delete from tbl20 where col = 'bar'", + "Instructions": { + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=Off) */ from tbl20 where col = 'bar'", + "Table": "tbl20", + "Values": [ + "'bar'" + ], + "Vindex": "hash_vin" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl20" + ] + } + }, + { + "comment": "Delete table with cross-shard foreign key with set null - should be eventually allowed", + "query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl9 where col9 = 34", + "plan": { + "QueryType": "DELETE", + "Original": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl9 where col9 = 34", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select col9 from tbl9 where 1 != 1", + "Query": "select col9 from tbl9 where col9 = 34 for update", + "Table": "tbl9", + "Values": [ + "34" + ], + "Vindex": "hash_vin" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set col_ref = null where (col_ref) in ::fkc_vals", + "Table": "tbl4" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl9 where col9 = 34", + "Table": "tbl9", + "Values": [ + "34" + ], + "Vindex": "hash_vin" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl4", + "sharded_fk_allow.tbl9" + ] + } + }, + { + "comment": "Delete with foreign key checks off", + "query": "delete /*+ SET_VAR(foreign_key_checks=off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "plan": { + "QueryType": "DELETE", + "Original": "delete /*+ SET_VAR(foreign_key_checks=off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Instructions": { + "OperatorType": "Delete", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=Off) */ from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3", + "Table": "multicol_tbl1", + "Values": [ + "1", + "2", + "3" + ], + "Vindex": "multicolIdx" + }, + "TablesUsed": [ + "sharded_fk_allow.multicol_tbl1" + ] + } + }, + { + "comment": "Update with foreign key checks off", + "query": "update /*+ SET_VAR(foreign_key_checks=0) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "plan": { + "QueryType": "UPDATE", + "Original": "update /*+ SET_VAR(foreign_key_checks=0) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Instructions": { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl1 set cola = 1, colb = 2 where id = 3", + "Table": "u_multicol_tbl1" + }, + "TablesUsed": [ + "unsharded_fk_allow.u_multicol_tbl1" + ] + } + }, + { + "comment": "Insert with cross shard foreign keys and foreign key checks off", + "query": "insert /*+ SET_VAR(foreign_key_checks=0) */ into tbl3 (col3, coly) values (1, 3)", + "plan": { + "QueryType": "INSERT", + "Original": "insert /*+ SET_VAR(foreign_key_checks=0) */ into tbl3 (col3, coly) values (1, 3)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Sharded", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "insert /*+ SET_VAR(foreign_key_checks=Off) */ into tbl3(col3, coly) values (:_col3_0, 3)", + "TableName": "tbl3", + "VindexValues": { + "hash_vin": "1" + } + }, + "TablesUsed": [ + "sharded_fk_allow.tbl3" + ] + } + } +] From 9d4263ee828424e42d2cc8fedeb556226335d587 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 9 Nov 2023 13:03:27 +0530 Subject: [PATCH 11/25] comments: add comments explaining the code Signed-off-by: Manan Gupta --- go/vt/sqlparser/comments.go | 64 ++++++++++++++++++++++++++++++++++-- go/vt/vtgate/vcursor_impl.go | 9 ++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index c5f3c4505c5..7a021abe0ea 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -274,23 +274,34 @@ func (c *ParsedComments) Directives() *CommentDirectives { // GetMySQLSetVarValue gets the value of the given variable if it is part of a /*+ SET_VAR() */ MySQL optimizer hint. func (c *ParsedComments) GetMySQLSetVarValue(key string) string { if c == nil { + // If we have no parsed comments, then we return an empty string. return "" } for _, commentStr := range c.comments { + // Skip all the comments that don't start with the query optimizer prefix. if commentStr[0:3] != queryOptimizerPrefix { continue } pos := 4 for pos < len(commentStr) { + // Go over the entire comment and extract an optimizer hint. + // We get back the final position of the cursor, along with the start and end of + // the optimizer hint name and content. finalPos, ohNameStart, ohNameEnd, ohContentStart, ohContentEnd := getOptimizerHint(pos, commentStr) pos = finalPos + 1 + // If we didn't find an optimizer hint or if it was malformed, we skip it. if ohContentEnd == -1 { break } + // Construct the name and the content from the starts and ends. ohName := commentStr[ohNameStart:ohNameEnd] ohContent := commentStr[ohContentStart:ohContentEnd] + // Check if the optimizer hint name matches `SET_VAR`. if strings.EqualFold(strings.TrimSpace(ohName), OptimizerHintSetVar) { + // If it does, then we cut the string at the first occurrence of "=". + // That gives us the name of the variable, and the value that it is being set to. + // If the variable matches what we are looking for, we return its value. setVarName, setVarValue, isValid := strings.Cut(ohContent, "=") if !isValid { continue @@ -301,6 +312,7 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { } } + // MySQL only parses the first comment that has the optimizer hint prefix. The following ones are ignored. return "" } return "" @@ -309,11 +321,15 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { // SetMySQLSetVarValue updates or sets the value of the given variable as part of a /*+ SET_VAR() */ MySQL optimizer hint. func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComments Comments) { if c == nil { + // If we have no parsed comments, then we create a new one with the required optimizer hint and return it. newComments = append(newComments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) return } seenFirstOhComment := false for _, commentStr := range c.comments { + // Skip all the comments that don't start with the query optimizer prefix. + // Also, since MySQL only parses the first comment that has the optimizer hint prefix and ignores the following ones, + // we skip over all the comments that come after we have seen the first comment with the optimizer hint. if seenFirstOhComment || commentStr[0:3] != queryOptimizerPrefix { newComments = append(newComments, commentStr) continue @@ -324,14 +340,24 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComme keyPresent := false pos := 4 for pos < len(commentStr) { + // Go over the entire comment and extract an optimizer hint. + // We get back the final position of the cursor, along with the start and end of + // the optimizer hint name and content. finalPos, ohNameStart, ohNameEnd, ohContentStart, ohContentEnd := getOptimizerHint(pos, commentStr) pos = finalPos + 1 + // If we didn't find an optimizer hint or if it was malformed, we skip it. if ohContentEnd == -1 { break } + // Construct the name and the content from the starts and ends. ohName := commentStr[ohNameStart:ohNameEnd] ohContent := commentStr[ohContentStart:ohContentEnd] + // Check if the optimizer hint name matches `SET_VAR`. if strings.EqualFold(strings.TrimSpace(ohName), OptimizerHintSetVar) { + // If it does, then we cut the string at the first occurrence of "=". + // That gives us the name of the variable, and the value that it is being set to. + // If the variable matches what we are looking for, we can change its value. + // Otherwise we add the comment as is to our final comments and move on. setVarName, _, isValid := strings.Cut(ohContent, "=") if !isValid || !strings.EqualFold(strings.TrimSpace(setVarName), key) { finalComment += fmt.Sprintf(" %v(%v)", ohName, ohContent) @@ -342,9 +368,12 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComme finalComment += fmt.Sprintf(" %v(%v=%v)", ohName, strings.TrimSpace(setVarName), value) } } else { + // If it doesn't match, we add it to our final comment and move on. finalComment += fmt.Sprintf(" %v(%v)", ohName, ohContent) } } + // If we haven't found any SET_VAR optimizer hint with the matching variable, + // then we add a new optimizer hint to introduce this variable. if !keyPresent { finalComment += fmt.Sprintf(" %v(%v=%v)", OptimizerHintSetVar, key, value) } @@ -352,60 +381,90 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComme finalComment += " */" newComments = append(newComments, finalComment) } + // If we have not seen even a single comment that has the optimizer hint prefix, + // then we add a new optimizer hint to introduce this variable. if !seenFirstOhComment { newComments = append(newComments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) } return newComments } +// getOptimizerHint goes over the comment string from the given initial position. +// It returns back the final position of the cursor, along with the start and end of +// the optimizer hint name and content. func getOptimizerHint(initialPos int, commentStr string) (pos int, ohNameStart int, ohNameEnd int, ohContentStart int, ohContentEnd int) { ohContentEnd = -1 + // skip spaces as they aren't interesting. pos = skipBlanks(initialPos, commentStr) ohNameStart = pos pos++ + // All characters until we get a space of a opening bracket are part of the optimizer hint name. for pos < len(commentStr) { if commentStr[pos] == ' ' || commentStr[pos] == '(' { break } pos++ } + // Mark the end of the optimizer hint name and skip spaces. ohNameEnd = pos pos = skipBlanks(pos, commentStr) + // Verify that the comment is not malformed. If it doesn't contain an opening bracket + // at the current position, then something is wrong. if pos >= len(commentStr) || commentStr[pos] != '(' { return } + // Seeing the opening bracket, marks the start of the optimizer hint content. + // We skip over the comment until we see the end of the parenthesis. pos++ ohContentStart = pos - pos = skipUntilParanthesisEnd(pos, commentStr) + pos = skipUntilParenthesisEnd(pos, commentStr) ohContentEnd = pos return } -func skipUntilParanthesisEnd(pos int, commentStr string) int { +// skipUntilParenthesisEnd reads the comment string given the initial position and skips over until +// it has seen the end of opening bracket. +func skipUntilParenthesisEnd(pos int, commentStr string) int { + // This is a problem of bracket matching. We solve this using a stack, because strings are also part of the comment + // that can also have '(' and ')' characters. So we can't just skip over until we see a closing parenthesis. + // Initialize a stack with opening bracket. stack := []byte{'('} for pos < len(commentStr) { switch commentStr[pos] { case '(': + // If we see an opening bracket, we put it into the stack + // only if we aren't currently reading a string. if stack[len(stack)-1] == '(' { stack = append(stack, '(') } case ')': + // If we see a closing bracket, we try to remove an opening + // bracket from the stack. if stack[len(stack)-1] == '(' { stack = stack[:len(stack)-1] } case '\'': + // If we see a single quote character, then it can either + // signify the ending of a previously started string, it could be + // part of a string that is encased in double quotes, or it could + // be the start of a new string. if stack[len(stack)-1] == '\'' { stack = stack[:len(stack)-1] } else if stack[len(stack)-1] != '"' { stack = append(stack, '\'') } case '"': + // If we see a double quote character, then it can either + // signify the ending of a previously started string, it could be + // part of a string that is encased in single quotes, or it could + // be the start of a new string. if stack[len(stack)-1] == '"' { stack = stack[:len(stack)-1] } else if stack[len(stack)-1] != '\'' { stack = append(stack, '"') } } + // The first time the stack becomes empty, signifies the ending of the parenthesis we set out to match. if len(stack) == 0 { return pos } @@ -415,6 +474,7 @@ func skipUntilParanthesisEnd(pos int, commentStr string) int { return pos } +// skipBlanks skips over space characters from the comment string, given the starting position. func skipBlanks(pos int, commentStr string) int { for pos < len(commentStr) { if commentStr[pos] == ' ' { diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index dc10cd2e8d0..b5a2c7aabdf 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -106,8 +106,11 @@ type vcursorImpl struct { logStats *logstats.LogStats collation collations.ID - ignoreMaxMemoryRows bool + // fkChecksState stores the state of foreign key checks variable. + // This state is meant to be the final fk checks state after consulting the + // session state, and the given query's comments for `SET_VAR` optimizer hints. fkChecksState sqlparser.FkChecksState + ignoreMaxMemoryRows bool vschema *vindexes.VSchema vm VSchemaOperator semTable *semantics.SemTable @@ -1348,11 +1351,15 @@ func (vc *vcursorImpl) CloneForReplicaWarming(ctx context.Context) engine.VCurso // UpdateForeignKeyChecksState updates the foreign key checks state of the vcursor. func (vc *vcursorImpl) UpdateForeignKeyChecksState(fkStateFromQuery sqlparser.FkChecksState) { + // Initialize the state to unspecified. vc.fkChecksState = sqlparser.FkChecksUnspecified + // If the query has a SET_VAR optimizer hint that explicitly sets the foreign key checks state, + // we should use that. if fkStateFromQuery != sqlparser.FkChecksUnspecified { vc.fkChecksState = fkStateFromQuery return } + // If the query doesn't have anything, then we consult the session state. fkVal, isPresent := vc.safeSession.SystemVariables[sysvars.ForeignKeyChecks.Name] if isPresent { switch strings.ToLower(fkVal) { From e2a3bc1036e96ddf677989feeb4174cea374b564 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 9 Nov 2023 13:22:48 +0530 Subject: [PATCH 12/25] test: fix test expectations Signed-off-by: Manan Gupta --- go/vt/vtgate/executor_set_test.go | 5 +++-- go/vt/vtgate/vcursor_impl.go | 2 ++ go/vt/vtgate/vcursor_impl_test.go | 20 ++++++++++++++++---- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index e71a41eeb7f..68898b7ea45 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -319,8 +319,9 @@ func TestExecutorSetOp(t *testing.T) { sysVars: map[string]string{"sql_quote_show_create": "0"}, result: returnResult("sql_quote_show_create", "int64", "0"), }, { - in: "set foreign_key_checks = 1", - result: returnNoResult("foreign_key_checks", "int64"), + in: "set foreign_key_checks = 1", + sysVars: map[string]string{"foreign_key_checks": "1"}, + result: returnNoResult("foreign_key_checks", "int64"), }, { in: "set unique_checks = 0", sysVars: map[string]string{"unique_checks": "0"}, diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index b5a2c7aabdf..c2706fc1867 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -1087,6 +1087,8 @@ func (vc *vcursorImpl) keyForPlan(ctx context.Context, query string, buf io.Stri _, _ = buf.WriteString(vindexes.TabletTypeSuffix[vc.tabletType]) _, _ = buf.WriteString("+Collate:") _, _ = buf.WriteString(collations.Local().LookupName(vc.collation)) + // The plans are going to be different based on the foreign key checks state. So we need to use that value + // as part of the plan's hashing key. _, _ = buf.WriteString("+fkChecksState:") _, _ = buf.WriteString(vc.GetForeignKeyChecksState().String()) diff --git a/go/vt/vtgate/vcursor_impl_test.go b/go/vt/vtgate/vcursor_impl_test.go index 3160b8a9b1a..60ca8a2c061 100644 --- a/go/vt/vtgate/vcursor_impl_test.go +++ b/go/vt/vtgate/vcursor_impl_test.go @@ -260,6 +260,7 @@ func TestSetTarget(t *testing.T) { func TestKeyForPlan(t *testing.T) { type testCase struct { vschema *vindexes.VSchema + fkState sqlparser.FkChecksState targetString string expectedPlanPrefixKey string } @@ -267,19 +268,29 @@ func TestKeyForPlan(t *testing.T) { tests := []testCase{{ vschema: vschemaWith1KS, targetString: "", - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1@replica", - expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+fkChecksState:+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1:-80", - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+DestinationShard(-80)+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:+DestinationShard(-80)+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1[deadbeef]", - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+KsIDsResolved:80-+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:+KsIDsResolved:80-+Query:SELECT 1", + }, { + vschema: vschemaWith1KS, + targetString: "", + fkState: sqlparser.FkChecksOn, + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:On+Query:SELECT 1", + }, { + vschema: vschemaWith1KS, + targetString: "ks1@replica", + fkState: sqlparser.FkChecksOff, + expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+fkChecksState:Off+Query:SELECT 1", }} for i, tc := range tests { @@ -289,6 +300,7 @@ func TestKeyForPlan(t *testing.T) { vc, err := newVCursorImpl(ss, sqlparser.MarginComments{}, nil, nil, &fakeVSchemaOperator{vschema: tc.vschema}, tc.vschema, srvtopo.NewResolver(&fakeTopoServer{}, nil, ""), nil, false, querypb.ExecuteOptions_Gen4) require.NoError(t, err) vc.vschema = tc.vschema + vc.fkChecksState = tc.fkState var buf strings.Builder vc.keyForPlan(context.Background(), "SELECT 1", &buf) From 990d9b1234397aeed111179c98227d740e1b4dc4 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 9 Nov 2023 14:43:09 +0530 Subject: [PATCH 13/25] test: augment the fuzzer to run different foreign key checks mode Signed-off-by: Manan Gupta --- .../vtgate/foreignkey/fk_fuzz_test.go | 102 +++++++++++------- 1 file changed, 64 insertions(+), 38 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index 134b9cfa180..73f60ac5055 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go @@ -33,6 +33,7 @@ import ( "vitess.io/vitess/go/test/endtoend/cluster" "vitess.io/vitess/go/test/endtoend/utils" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/sqlparser" ) type QueryFormat string @@ -128,17 +129,18 @@ func (fz *fuzzer) generateInsertDMLQuery() string { tableId := rand.Intn(len(fkTables)) idValue := 1 + rand.Intn(fz.maxValForId) tableName := fkTables[tableId] + setVarFkChecksVal := fz.getSetVarFkChecksVal() if tableName == "fk_t20" { colValue := rand.Intn(1 + fz.maxValForCol) col2Value := rand.Intn(1 + fz.maxValForCol) - return fmt.Sprintf("insert into %v (id, col, col2) values (%v, %v, %v)", tableName, idValue, convertColValueToString(colValue), convertColValueToString(col2Value)) + return fmt.Sprintf("insert %vinto %v (id, col, col2) values (%v, %v, %v)", setVarFkChecksVal, tableName, idValue, convertColValueToString(colValue), convertColValueToString(col2Value)) } else if isMultiColFkTable(tableName) { colaValue := rand.Intn(1 + fz.maxValForCol) colbValue := rand.Intn(1 + fz.maxValForCol) - return fmt.Sprintf("insert into %v (id, cola, colb) values (%v, %v, %v)", tableName, idValue, convertColValueToString(colaValue), convertColValueToString(colbValue)) + return fmt.Sprintf("insert %vinto %v (id, cola, colb) values (%v, %v, %v)", setVarFkChecksVal, tableName, idValue, convertColValueToString(colaValue), convertColValueToString(colbValue)) } else { colValue := rand.Intn(1 + fz.maxValForCol) - return fmt.Sprintf("insert into %v (id, col) values (%v, %v)", tableName, idValue, convertColValueToString(colValue)) + return fmt.Sprintf("insert %vinto %v (id, col) values (%v, %v)", setVarFkChecksVal, tableName, idValue, convertColValueToString(colValue)) } } @@ -155,17 +157,18 @@ func (fz *fuzzer) generateUpdateDMLQuery() string { tableId := rand.Intn(len(fkTables)) idValue := 1 + rand.Intn(fz.maxValForId) tableName := fkTables[tableId] + setVarFkChecksVal := fz.getSetVarFkChecksVal() if tableName == "fk_t20" { colValue := rand.Intn(1 + fz.maxValForCol) col2Value := rand.Intn(1 + fz.maxValForCol) - return fmt.Sprintf("update %v set col = %v, col2 = %v where id = %v", tableName, convertColValueToString(colValue), convertColValueToString(col2Value), idValue) + return fmt.Sprintf("update %v%v set col = %v, col2 = %v where id = %v", setVarFkChecksVal, tableName, convertColValueToString(colValue), convertColValueToString(col2Value), idValue) } else if isMultiColFkTable(tableName) { colaValue := rand.Intn(1 + fz.maxValForCol) colbValue := rand.Intn(1 + fz.maxValForCol) - return fmt.Sprintf("update %v set cola = %v, colb = %v where id = %v", tableName, convertColValueToString(colaValue), convertColValueToString(colbValue), idValue) + return fmt.Sprintf("update %v%v set cola = %v, colb = %v where id = %v", setVarFkChecksVal, tableName, convertColValueToString(colaValue), convertColValueToString(colbValue), idValue) } else { colValue := rand.Intn(1 + fz.maxValForCol) - return fmt.Sprintf("update %v set col = %v where id = %v", tableName, convertColValueToString(colValue), idValue) + return fmt.Sprintf("update %v%v set col = %v where id = %v", setVarFkChecksVal, tableName, convertColValueToString(colValue), idValue) } } @@ -173,7 +176,8 @@ func (fz *fuzzer) generateUpdateDMLQuery() string { func (fz *fuzzer) generateDeleteDMLQuery() string { tableId := rand.Intn(len(fkTables)) idValue := 1 + rand.Intn(fz.maxValForId) - query := fmt.Sprintf("delete from %v where id = %v", fkTables[tableId], idValue) + setVarFkChecksVal := fz.getSetVarFkChecksVal() + query := fmt.Sprintf("delete %vfrom %v where id = %v", setVarFkChecksVal, fkTables[tableId], idValue) return query } @@ -456,6 +460,21 @@ func (fz *fuzzer) generateParameterizedDeleteQuery() (query string, params []any return fmt.Sprintf("delete from %v where id = ?", fkTables[tableId]), []any{idValue} } +// getSetVarFkChecksVal generates an optimizer hint to randomly set the foreign key checks to on or off or leave them unaltered. +func (fz *fuzzer) getSetVarFkChecksVal() string { + if fz.concurrency != 1 { + return "" + } + val := rand.Intn(3) + if val == 0 { + return "" + } + if val == 1 { + return "/*+ SET_VAR(foreign_key_checks=On) */ " + } + return "/*+ SET_VAR(foreign_key_checks=Off) */ " +} + // TestFkFuzzTest is a fuzzer test that works by querying the database concurrently. // We have a pre-written set of query templates that we will use, but the data in the queries will // be randomly generated. The intent is that we hammer the database as a real-world application would @@ -607,45 +626,52 @@ func TestFkFuzzTest(t *testing.T) { for _, tt := range testcases { for _, testSharded := range []bool{false, true} { for _, queryFormat := range []QueryFormat{SQLQueries, PreparedStatmentQueries, PreparedStatementPacket} { - t.Run(getTestName(tt.name, testSharded)+fmt.Sprintf(" QueryFormat - %v", queryFormat), func(t *testing.T) { - mcmp, closer := start(t) - defer closer() - // Set the correct keyspace to use from VtGates. - if testSharded { - t.Skip("Skip test since we don't have sharded foreign key support yet") - _ = utils.Exec(t, mcmp.VtConn, "use `ks`") - } else { - _ = utils.Exec(t, mcmp.VtConn, "use `uks`") + for _, fkState := range []sqlparser.FkChecksState{sqlparser.FkChecksOn, sqlparser.FkChecksOff} { + if fkState == sqlparser.FkChecksOff && (queryFormat != SQLQueries || tt.concurrency != 1) { + continue } - // Ensure that the Vitess database is originally empty - ensureDatabaseState(t, mcmp.VtConn, true) - ensureDatabaseState(t, mcmp.MySQLConn, true) + t.Run(getTestName(tt.name, testSharded)+fmt.Sprintf(" FkState - %v QueryFormat - %v", fkState.String(), queryFormat), func(t *testing.T) { + mcmp, closer := start(t) + defer closer() + // Set the correct keyspace to use from VtGates. + if testSharded { + t.Skip("Skip test since we don't have sharded foreign key support yet") + _ = utils.Exec(t, mcmp.VtConn, "use `ks`") + } else { + _ = utils.Exec(t, mcmp.VtConn, "use `uks`") + } + mcmp.Exec(fmt.Sprintf("SET GLOBAL FOREIGN_KEY_CHECKS=%v", fkState.String())) + + // Ensure that the Vitess database is originally empty + ensureDatabaseState(t, mcmp.VtConn, true) + ensureDatabaseState(t, mcmp.MySQLConn, true) - // Create the fuzzer. - fz := newFuzzer(tt.concurrency, tt.maxValForId, tt.maxValForCol, tt.insertShare, tt.deleteShare, tt.updateShare, queryFormat) + // Create the fuzzer. + fz := newFuzzer(tt.concurrency, tt.maxValForId, tt.maxValForCol, tt.insertShare, tt.deleteShare, tt.updateShare, queryFormat) - // Start the fuzzer. - fz.start(t, testSharded) + // Start the fuzzer. + fz.start(t, testSharded) - // Wait for the timeForTesting so that the threads continue to run. - time.Sleep(tt.timeForTesting) + // Wait for the timeForTesting so that the threads continue to run. + time.Sleep(tt.timeForTesting) - fz.stop() + fz.stop() - // We encountered an error while running the fuzzer. Let's print out the information! - if fz.firstFailureInfo != nil { - log.Errorf("Failing query - %v", fz.firstFailureInfo.queryToFail) - for idx, table := range fkTables { - log.Errorf("MySQL data for %v -\n%v", table, fz.firstFailureInfo.mysqlState[idx].Rows) - log.Errorf("Vitess data for %v -\n%v", table, fz.firstFailureInfo.vitessState[idx].Rows) + // We encountered an error while running the fuzzer. Let's print out the information! + if fz.firstFailureInfo != nil { + log.Errorf("Failing query - %v", fz.firstFailureInfo.queryToFail) + for idx, table := range fkTables { + log.Errorf("MySQL data for %v -\n%v", table, fz.firstFailureInfo.mysqlState[idx].Rows) + log.Errorf("Vitess data for %v -\n%v", table, fz.firstFailureInfo.vitessState[idx].Rows) + } } - } - // ensure Vitess database has some data. This ensures not all the commands failed. - ensureDatabaseState(t, mcmp.VtConn, false) - // Verify the consistency of the data. - verifyDataIsCorrect(t, mcmp, tt.concurrency) - }) + // ensure Vitess database has some data. This ensures not all the commands failed. + ensureDatabaseState(t, mcmp.VtConn, false) + // Verify the consistency of the data. + verifyDataIsCorrect(t, mcmp, tt.concurrency) + }) + } } } } From 3456849888146044cd4cb1528f0914bbfb5127af Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 9 Nov 2023 15:10:00 +0530 Subject: [PATCH 14/25] fuzzer: fix fuzzer Signed-off-by: Manan Gupta --- .../vtgate/foreignkey/fk_fuzz_test.go | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index 73f60ac5055..91e7c3ee7c8 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go @@ -54,6 +54,7 @@ type fuzzer struct { updateShare int concurrency int queryFormat QueryFormat + fkState sqlparser.FkChecksState // shouldStop is an internal state variable, that tells the fuzzer // whether it should stop or not. @@ -73,7 +74,7 @@ type debugInfo struct { } // newFuzzer creates a new fuzzer struct. -func newFuzzer(concurrency int, maxValForId int, maxValForCol int, insertShare int, deleteShare int, updateShare int, queryFormat QueryFormat) *fuzzer { +func newFuzzer(concurrency int, maxValForId int, maxValForCol int, insertShare int, deleteShare int, updateShare int, queryFormat QueryFormat, fkState sqlparser.FkChecksState) *fuzzer { fz := &fuzzer{ concurrency: concurrency, maxValForId: maxValForId, @@ -82,6 +83,7 @@ func newFuzzer(concurrency int, maxValForId int, maxValForCol int, insertShare i deleteShare: deleteShare, updateShare: updateShare, queryFormat: queryFormat, + fkState: fkState, wg: sync.WaitGroup{}, } // Initially the fuzzer thread is stopped. @@ -203,6 +205,9 @@ func (fz *fuzzer) runFuzzerThread(t *testing.T, sharded bool, fuzzerThreadId int // Create a MySQL Compare that connects to both Vitess and MySQL and runs the queries against both. mcmp, err := utils.NewMySQLCompare(t, vtParams, mysqlParams) require.NoError(t, err) + if fz.fkState != sqlparser.FkChecksUnspecified { + mcmp.Exec(fmt.Sprintf("SET FOREIGN_KEY_CHECKS=%v", fz.fkState.String())) + } var vitessDb, mysqlDb *sql.DB if fz.queryFormat == PreparedStatementPacket { // Open another connection to Vitess using the go-sql-driver so that we can send prepared statements as COM_STMT_PREPARE packets. @@ -622,12 +627,11 @@ func TestFkFuzzTest(t *testing.T) { updateShare: 50, }, } - - for _, tt := range testcases { - for _, testSharded := range []bool{false, true} { - for _, queryFormat := range []QueryFormat{SQLQueries, PreparedStatmentQueries, PreparedStatementPacket} { - for _, fkState := range []sqlparser.FkChecksState{sqlparser.FkChecksOn, sqlparser.FkChecksOff} { - if fkState == sqlparser.FkChecksOff && (queryFormat != SQLQueries || tt.concurrency != 1) { + for _, fkState := range []sqlparser.FkChecksState{sqlparser.FkChecksUnspecified, sqlparser.FkChecksOn, sqlparser.FkChecksOff} { + for _, tt := range testcases { + for _, testSharded := range []bool{false, true} { + for _, queryFormat := range []QueryFormat{SQLQueries, PreparedStatmentQueries, PreparedStatementPacket} { + if fkState != sqlparser.FkChecksUnspecified && (queryFormat != SQLQueries || tt.concurrency != 1) { continue } t.Run(getTestName(tt.name, testSharded)+fmt.Sprintf(" FkState - %v QueryFormat - %v", fkState.String(), queryFormat), func(t *testing.T) { @@ -640,14 +644,13 @@ func TestFkFuzzTest(t *testing.T) { } else { _ = utils.Exec(t, mcmp.VtConn, "use `uks`") } - mcmp.Exec(fmt.Sprintf("SET GLOBAL FOREIGN_KEY_CHECKS=%v", fkState.String())) // Ensure that the Vitess database is originally empty ensureDatabaseState(t, mcmp.VtConn, true) ensureDatabaseState(t, mcmp.MySQLConn, true) // Create the fuzzer. - fz := newFuzzer(tt.concurrency, tt.maxValForId, tt.maxValForCol, tt.insertShare, tt.deleteShare, tt.updateShare, queryFormat) + fz := newFuzzer(tt.concurrency, tt.maxValForId, tt.maxValForCol, tt.insertShare, tt.deleteShare, tt.updateShare, queryFormat, fkState) // Start the fuzzer. fz.start(t, testSharded) @@ -727,7 +730,7 @@ func verifyDataIsCorrect(t *testing.T, mcmp utils.MySQLCompare, concurrency int) require.NotNil(t, primaryTab) require.NotNil(t, replicaTab) checkReplicationHealthy(t, replicaTab) - cluster.WaitForReplicationPos(t, primaryTab, replicaTab, true, 60.0) + cluster.WaitForReplicationPos(t, primaryTab, replicaTab, true, 1*time.Minute) primaryConn, err := utils.GetMySQLConn(primaryTab, fmt.Sprintf("vt_%v", keyspace.Name)) require.NoError(t, err) replicaConn, err := utils.GetMySQLConn(replicaTab, fmt.Sprintf("vt_%v", keyspace.Name)) From ec3629ef4e41a864fdfec4919893fc5544935dc4 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 13 Nov 2023 15:00:01 +0530 Subject: [PATCH 15/25] feat: only add fkChecksState field to the plan key if it is non-empty Signed-off-by: Manan Gupta --- go/vt/vtgate/vcursor_impl.go | 7 +++++-- go/vt/vtgate/vcursor_impl_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index c2706fc1867..fbc6fd54868 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -1089,8 +1089,11 @@ func (vc *vcursorImpl) keyForPlan(ctx context.Context, query string, buf io.Stri _, _ = buf.WriteString(collations.Local().LookupName(vc.collation)) // The plans are going to be different based on the foreign key checks state. So we need to use that value // as part of the plan's hashing key. - _, _ = buf.WriteString("+fkChecksState:") - _, _ = buf.WriteString(vc.GetForeignKeyChecksState().String()) + fkChecksState := vc.GetForeignKeyChecksState().String() + if fkChecksState != "" { + _, _ = buf.WriteString("+fkChecksState:") + _, _ = buf.WriteString(fkChecksState) + } if vc.destination != nil { switch vc.destination.(type) { diff --git a/go/vt/vtgate/vcursor_impl_test.go b/go/vt/vtgate/vcursor_impl_test.go index 60ca8a2c061..2a462b06a27 100644 --- a/go/vt/vtgate/vcursor_impl_test.go +++ b/go/vt/vtgate/vcursor_impl_test.go @@ -268,19 +268,19 @@ func TestKeyForPlan(t *testing.T) { tests := []testCase{{ vschema: vschemaWith1KS, targetString: "", - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1@replica", - expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+fkChecksState:+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1:-80", - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:+DestinationShard(-80)+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+DestinationShard(-80)+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1[deadbeef]", - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:+KsIDsResolved:80-+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+KsIDsResolved:80-+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "", From 0d412dd593ce2e9d7c4795a3bfcb81510f9fe65a Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 13 Nov 2023 16:48:05 +0530 Subject: [PATCH 16/25] feat: remove sqlparser.FkChecksState and instead use a *bool to store them Signed-off-by: Manan Gupta --- .../vtgate/foreignkey/fk_fuzz_test.go | 17 ++++++++------ go/test/vschemawrapper/vschema_wrapper.go | 4 ++-- go/vt/schemadiff/semantics.go | 4 ++-- go/vt/sqlparser/ast_funcs.go | 12 ++++++---- go/vt/sqlparser/ast_rewriting.go | 14 +++++------ go/vt/sqlparser/ast_rewriting_test.go | 8 +++---- go/vt/sqlparser/comments.go | 10 ++++---- go/vt/sqlparser/constants.go | 9 -------- go/vt/sqlparser/normalizer_test.go | 4 ++-- go/vt/vtgate/planbuilder/builder.go | 2 +- go/vt/vtgate/planbuilder/operators/delete.go | 5 ++-- go/vt/vtgate/planbuilder/operators/update.go | 7 +++--- go/vt/vtgate/planbuilder/plan_test.go | 6 +++-- .../vtgate/planbuilder/plancontext/vschema.go | 2 +- go/vt/vtgate/planbuilder/simplifier_test.go | 8 +++---- go/vt/vtgate/semantics/FakeSI.go | 4 ++-- go/vt/vtgate/semantics/analyzer.go | 6 ++--- go/vt/vtgate/semantics/analyzer_fk_test.go | 3 ++- go/vt/vtgate/semantics/info_schema.go | 4 ++-- go/vt/vtgate/semantics/semantic_state.go | 2 +- go/vt/vtgate/vcursor_impl.go | 23 +++++++++++-------- go/vt/vtgate/vcursor_impl_test.go | 8 ++++--- 22 files changed, 86 insertions(+), 76 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index 9fa5b828fbb..cfa9ad36485 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go @@ -53,7 +53,7 @@ type fuzzer struct { updateShare int concurrency int queryFormat QueryFormat - fkState sqlparser.FkChecksState + fkState *bool // shouldStop is an internal state variable, that tells the fuzzer // whether it should stop or not. @@ -73,7 +73,7 @@ type debugInfo struct { } // newFuzzer creates a new fuzzer struct. -func newFuzzer(concurrency int, maxValForId int, maxValForCol int, insertShare int, deleteShare int, updateShare int, queryFormat QueryFormat, fkState sqlparser.FkChecksState) *fuzzer { +func newFuzzer(concurrency int, maxValForId int, maxValForCol int, insertShare int, deleteShare int, updateShare int, queryFormat QueryFormat, fkState *bool) *fuzzer { fz := &fuzzer{ concurrency: concurrency, maxValForId: maxValForId, @@ -206,8 +206,8 @@ func (fz *fuzzer) runFuzzerThread(t *testing.T, sharded bool, fuzzerThreadId int // Create a MySQL Compare that connects to both Vitess and MySQL and runs the queries against both. mcmp, err := utils.NewMySQLCompare(t, vtParams, mysqlParams) require.NoError(t, err) - if fz.fkState != sqlparser.FkChecksUnspecified { - mcmp.Exec(fmt.Sprintf("SET FOREIGN_KEY_CHECKS=%v", fz.fkState.String())) + if fz.fkState != nil { + mcmp.Exec(fmt.Sprintf("SET FOREIGN_KEY_CHECKS=%v", sqlparser.FkChecksStateString(fz.fkState))) } var vitessDb, mysqlDb *sql.DB if fz.queryFormat == PreparedStatementPacket { @@ -631,14 +631,17 @@ func TestFkFuzzTest(t *testing.T) { updateShare: 50, }, } - for _, fkState := range []sqlparser.FkChecksState{sqlparser.FkChecksUnspecified, sqlparser.FkChecksOn, sqlparser.FkChecksOff} { + + valTrue := true + valFalse := false + for _, fkState := range []*bool{nil, &valTrue, &valFalse} { for _, tt := range testcases { for _, testSharded := range []bool{false, true} { for _, queryFormat := range []QueryFormat{OlapSQLQueries, SQLQueries, PreparedStatmentQueries, PreparedStatementPacket} { - if fkState != sqlparser.FkChecksUnspecified && (queryFormat != SQLQueries || tt.concurrency != 1) { + if fkState != nil && (queryFormat != SQLQueries || tt.concurrency != 1) { continue } - t.Run(getTestName(tt.name, testSharded)+fmt.Sprintf(" FkState - %v QueryFormat - %v", fkState.String(), queryFormat), func(t *testing.T) { + t.Run(getTestName(tt.name, testSharded)+fmt.Sprintf(" FkState - %v QueryFormat - %v", sqlparser.FkChecksStateString(fkState), queryFormat), func(t *testing.T) { mcmp, closer := start(t) defer closer() // Set the correct keyspace to use from VtGates. diff --git a/go/test/vschemawrapper/vschema_wrapper.go b/go/test/vschemawrapper/vschema_wrapper.go index 04cedba581a..85d9840c3f7 100644 --- a/go/test/vschemawrapper/vschema_wrapper.go +++ b/go/test/vschemawrapper/vschema_wrapper.go @@ -45,7 +45,7 @@ type VSchemaWrapper struct { TabletType_ topodatapb.TabletType Dest key.Destination SysVarEnabled bool - ForeignKeyChecksState sqlparser.FkChecksState + ForeignKeyChecksState *bool Version plancontext.PlannerVersion EnableViews bool TestBuilder func(query string, vschema plancontext.VSchema, keyspace string) (*engine.Plan, error) @@ -141,7 +141,7 @@ func (vw *VSchemaWrapper) KeyspaceError(keyspace string) error { return nil } -func (vw *VSchemaWrapper) GetForeignKeyChecksState() sqlparser.FkChecksState { +func (vw *VSchemaWrapper) GetForeignKeyChecksState() *bool { return vw.ForeignKeyChecksState } diff --git a/go/vt/schemadiff/semantics.go b/go/vt/schemadiff/semantics.go index c2c4580ba28..7cfe127cf57 100644 --- a/go/vt/schemadiff/semantics.go +++ b/go/vt/schemadiff/semantics.go @@ -64,8 +64,8 @@ func (si *declarativeSchemaInformation) KeyspaceError(keyspace string) error { return nil } -func (si *declarativeSchemaInformation) GetForeignKeyChecksState() sqlparser.FkChecksState { - return sqlparser.FkChecksUnspecified +func (si *declarativeSchemaInformation) GetForeignKeyChecksState() *bool { + return nil } // addTable adds a fake table with an empty column list diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 218a2b0da9c..746830dbbce 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -357,11 +357,15 @@ func (node *ParsedComments) AddQueryHint(queryHint string) (Comments, error) { return newComments, nil } -func (s FkChecksState) String() string { - switch s { - case FkChecksOff: +// FkChecksStateString prints the foreign key checks state. +func FkChecksStateString(state *bool) string { + if state == nil { + return "" + } + switch *state { + case false: return "Off" - case FkChecksOn: + case true: return "On" } return "" diff --git a/go/vt/sqlparser/ast_rewriting.go b/go/vt/sqlparser/ast_rewriting.go index 3bfefeb4991..e20a5b80f70 100644 --- a/go/vt/sqlparser/ast_rewriting.go +++ b/go/vt/sqlparser/ast_rewriting.go @@ -51,7 +51,7 @@ func PrepareAST( selectLimit int, setVarComment string, sysVars map[string]string, - fkChecksState FkChecksState, + fkChecksState *bool, views VSchemaViews, ) (*RewriteASTResult, error) { if parameterize { @@ -71,7 +71,7 @@ func RewriteAST( selectLimit int, setVarComment string, sysVars map[string]string, - fkChecksState FkChecksState, + fkChecksState *bool, views VSchemaViews, ) (*RewriteASTResult, error) { er := newASTRewriter(keyspace, selectLimit, setVarComment, sysVars, fkChecksState, views) @@ -123,12 +123,12 @@ type astRewriter struct { keyspace string selectLimit int setVarComment string - fkChecksState FkChecksState + fkChecksState *bool sysVars map[string]string views VSchemaViews } -func newASTRewriter(keyspace string, selectLimit int, setVarComment string, sysVars map[string]string, fkChecksState FkChecksState, views VSchemaViews) *astRewriter { +func newASTRewriter(keyspace string, selectLimit int, setVarComment string, sysVars map[string]string, fkChecksState *bool, views VSchemaViews) *astRewriter { return &astRewriter{ bindVars: &BindVarNeeds{}, keyspace: keyspace, @@ -158,7 +158,7 @@ const ( ) func (er *astRewriter) rewriteAliasedExpr(node *AliasedExpr) (*BindVarNeeds, error) { - inner := newASTRewriter(er.keyspace, er.selectLimit, er.setVarComment, er.sysVars, FkChecksUnspecified, er.views) + inner := newASTRewriter(er.keyspace, er.selectLimit, er.setVarComment, er.sysVars, nil, er.views) inner.shouldRewriteDatabaseFunc = er.shouldRewriteDatabaseFunc tmp := SafeRewrite(node.Expr, inner.rewriteDown, inner.rewriteUp) newExpr, ok := tmp.(Expr) @@ -190,8 +190,8 @@ func (er *astRewriter) rewriteUp(cursor *Cursor) bool { } supportOptimizerHint.SetComments(newComments) } - if er.fkChecksState != FkChecksUnspecified { - newComments := supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, er.fkChecksState.String()) + if er.fkChecksState != nil { + newComments := supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, FkChecksStateString(er.fkChecksState)) supportOptimizerHint.SetComments(newComments) } } diff --git a/go/vt/sqlparser/ast_rewriting_test.go b/go/vt/sqlparser/ast_rewriting_test.go index 091546e8b6c..c8bc0fdbef9 100644 --- a/go/vt/sqlparser/ast_rewriting_test.go +++ b/go/vt/sqlparser/ast_rewriting_test.go @@ -348,7 +348,7 @@ func TestRewrites(in *testing.T) { SQLSelectLimitUnset, "", nil, - FkChecksUnspecified, + nil, &fakeViews{}, ) require.NoError(err) @@ -440,7 +440,7 @@ func TestRewritesWithSetVarComment(in *testing.T) { stmt, err := Parse(tc.in) require.NoError(err) - result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, tc.setVarComment, nil, FkChecksUnspecified, &fakeViews{}) + result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, tc.setVarComment, nil, nil, &fakeViews{}) require.NoError(err) expected, err := Parse(tc.expected) @@ -488,7 +488,7 @@ func TestRewritesSysVar(in *testing.T) { stmt, err := Parse(tc.in) require.NoError(err) - result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, "", tc.sysVar, FkChecksUnspecified, &fakeViews{}) + result, err := RewriteAST(stmt, "ks", SQLSelectLimitUnset, "", tc.sysVar, nil, &fakeViews{}) require.NoError(err) expected, err := Parse(tc.expected) @@ -538,7 +538,7 @@ func TestRewritesWithDefaultKeyspace(in *testing.T) { stmt, err := Parse(tc.in) require.NoError(err) - result, err := RewriteAST(stmt, "sys", SQLSelectLimitUnset, "", nil, FkChecksUnspecified, &fakeViews{}) + result, err := RewriteAST(stmt, "sys", SQLSelectLimitUnset, "", nil, nil, &fakeViews{}) require.NoError(err) expected, err := Parse(tc.expected) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 7a021abe0ea..e14a7b7be85 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -570,18 +570,20 @@ func AllowScatterDirective(stmt Statement) bool { } // ForeignKeyChecksState returns the state of foreign_key_checks variable if it is part of a SET_VAR optimizer hint in the comments. -func ForeignKeyChecksState(stmt Statement) FkChecksState { +func ForeignKeyChecksState(stmt Statement) *bool { cmt, ok := stmt.(Commented) if ok { fkChecksVal := cmt.GetParsedComments().GetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name) switch strings.ToLower(fkChecksVal) { case "on", "1": - return FkChecksOn + fkState := true + return &fkState case "off", "0": - return FkChecksOff + fkState := false + return &fkState } } - return FkChecksUnspecified + return nil } func checkDirective(stmt Statement, key string) bool { diff --git a/go/vt/sqlparser/constants.go b/go/vt/sqlparser/constants.go index b6d1e159e09..30effe51586 100644 --- a/go/vt/sqlparser/constants.go +++ b/go/vt/sqlparser/constants.go @@ -1072,12 +1072,3 @@ const ( IndexTypeSpatial IndexTypeFullText ) - -// FkChecksState is the enum to store the value of `foreign_key_checks` MySQL variable. -type FkChecksState int - -const ( - FkChecksUnspecified FkChecksState = iota - FkChecksOn - FkChecksOff -) diff --git a/go/vt/sqlparser/normalizer_test.go b/go/vt/sqlparser/normalizer_test.go index 67b5e8a6e9b..4582878d3ce 100644 --- a/go/vt/sqlparser/normalizer_test.go +++ b/go/vt/sqlparser/normalizer_test.go @@ -573,7 +573,7 @@ func BenchmarkNormalizeVTGate(b *testing.B) { SQLSelectLimitUnset, "", nil, /*sysvars*/ - FkChecksUnspecified, + nil, nil, /*views*/ ) if err != nil { @@ -864,7 +864,7 @@ func benchmarkNormalization(b *testing.B, sqls []string) { SQLSelectLimitUnset, "", nil, - FkChecksUnspecified, + nil, nil, ) if err != nil { diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index 0d1209e8732..f715c230db9 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -79,7 +79,7 @@ func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*e vw, isVw := vschema.(*vschemawrapper.VSchemaWrapper) if isVw { fkState := sqlparser.ForeignKeyChecksState(stmt) - if fkState != sqlparser.FkChecksUnspecified { + if fkState != nil { // Restore the old volue of ForeignKeyChecksState to not interfere with the next test cases. oldVal := vw.ForeignKeyChecksState vw.ForeignKeyChecksState = fkState diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index c09b2f72758..bdf38a22195 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -211,8 +211,9 @@ func createFkChildForDelete(ctx *plancontext.PlanningContext, fk vindexes.ChildF // We run with foreign key checks on because the delete might still fail on MySQL due to a child table // with RESTRICT constraints. var parsedComments *sqlparser.ParsedComments - if ctx.VSchema.GetForeignKeyChecksState() == sqlparser.FkChecksOn { - parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, sqlparser.FkChecksOn.String()).Parsed() + fkChecksState := ctx.VSchema.GetForeignKeyChecksState() + if fkChecksState != nil && *fkChecksState { + parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "On").Parsed() } var childStmt sqlparser.Statement switch fk.OnDelete { diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 66bcdadfeae..d43c414eb98 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -430,7 +430,7 @@ func buildChildUpdOpForCascade(ctx *plancontext.PlanningContext, fk vindexes.Chi // Because we could be updating the child to a non-null value, // We have to run with foreign key checks OFF because the parent isn't guaranteed to have // the data being updated to. - parsedComments := (&sqlparser.ParsedComments{}).SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, sqlparser.FkChecksOff.String()).Parsed() + parsedComments := (&sqlparser.ParsedComments{}).SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "Off").Parsed() childUpdStmt := &sqlparser.Update{ Comments: parsedComments, Exprs: childUpdateExprs, @@ -487,8 +487,9 @@ func buildChildUpdOpForSetNull( // We run with foreign key checks on because the update might still fail on MySQL due to a child table // with RESTRICT constraints. var parsedComments *sqlparser.ParsedComments - if ctx.VSchema.GetForeignKeyChecksState() == sqlparser.FkChecksOn { - parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, sqlparser.FkChecksOn.String()).Parsed() + fkState := ctx.VSchema.GetForeignKeyChecksState() + if fkState != nil && *fkState { + parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "On").Parsed() } childUpdStmt := &sqlparser.Update{ Exprs: childUpdateExprs, diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 9b06fee7b94..aec1c37e608 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -117,10 +117,11 @@ func TestForeignKeyPlanning(t *testing.T) { func TestForeignKeyChecksOn(t *testing.T) { vschema := loadSchema(t, "vschemas/schema.json", true) setFks(t, vschema) + fkChecksState := true vschemaWrapper := &vschemawrapper.VSchemaWrapper{ V: vschema, TestBuilder: TestBuilder, - ForeignKeyChecksState: sqlparser.FkChecksOn, + ForeignKeyChecksState: &fkChecksState, } testOutputTempDir := makeTestOutput(t) @@ -132,10 +133,11 @@ func TestForeignKeyChecksOn(t *testing.T) { func TestForeignKeyChecksOff(t *testing.T) { vschema := loadSchema(t, "vschemas/schema.json", true) setFks(t, vschema) + fkChecksState := false vschemaWrapper := &vschemawrapper.VSchemaWrapper{ V: vschema, TestBuilder: TestBuilder, - ForeignKeyChecksState: sqlparser.FkChecksOff, + ForeignKeyChecksState: &fkChecksState, } testOutputTempDir := makeTestOutput(t) diff --git a/go/vt/vtgate/planbuilder/plancontext/vschema.go b/go/vt/vtgate/planbuilder/plancontext/vschema.go index 10fbd4c4e80..286e8d30b67 100644 --- a/go/vt/vtgate/planbuilder/plancontext/vschema.go +++ b/go/vt/vtgate/planbuilder/plancontext/vschema.go @@ -60,7 +60,7 @@ type VSchema interface { // KeyspaceError returns any error in the keyspace vschema. KeyspaceError(keyspace string) error - GetForeignKeyChecksState() sqlparser.FkChecksState + GetForeignKeyChecksState() *bool // GetVSchema returns the latest cached vindexes.VSchema GetVSchema() *vindexes.VSchema diff --git a/go/vt/vtgate/planbuilder/simplifier_test.go b/go/vt/vtgate/planbuilder/simplifier_test.go index 5f39894a3ec..4c83af77933 100644 --- a/go/vt/vtgate/planbuilder/simplifier_test.go +++ b/go/vt/vtgate/planbuilder/simplifier_test.go @@ -48,7 +48,7 @@ func TestSimplifyBuggyQuery(t *testing.T) { } stmt, reserved, err := sqlparser.Parse2(query) require.NoError(t, err) - rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) + rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) reservedVars := sqlparser.NewReservedVars("vtg", reserved) simplified := simplifier.SimplifyStatement( @@ -70,7 +70,7 @@ func TestSimplifyPanic(t *testing.T) { } stmt, reserved, err := sqlparser.Parse2(query) require.NoError(t, err) - rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) + rewritten, _ := sqlparser.RewriteAST(sqlparser.CloneStatement(stmt), vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) reservedVars := sqlparser.NewReservedVars("vtg", reserved) simplified := simplifier.SimplifyStatement( @@ -100,7 +100,7 @@ func TestUnsupportedFile(t *testing.T) { t.Skip() return } - rewritten, err := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) + rewritten, err := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) if err != nil { t.Skip() } @@ -134,7 +134,7 @@ func keepSameError(query string, reservedVars *sqlparser.ReservedVars, vschema * if err != nil { panic(err) } - rewritten, _ := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, sqlparser.FkChecksUnspecified, nil) + rewritten, _ := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil) ast := rewritten.AST _, expected := BuildFromStmt(context.Background(), query, ast, reservedVars, vschema, rewritten.BindVarNeeds, true, true) if expected == nil { diff --git a/go/vt/vtgate/semantics/FakeSI.go b/go/vt/vtgate/semantics/FakeSI.go index f085d9ea92e..5a91ece816d 100644 --- a/go/vt/vtgate/semantics/FakeSI.go +++ b/go/vt/vtgate/semantics/FakeSI.go @@ -61,8 +61,8 @@ func (s *FakeSI) ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyM return vschemapb.Keyspace_unmanaged, nil } -func (s *FakeSI) GetForeignKeyChecksState() sqlparser.FkChecksState { - return sqlparser.FkChecksUnspecified +func (s *FakeSI) GetForeignKeyChecksState() *bool { + return nil } func (s *FakeSI) KeyspaceError(keyspace string) error { diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index 4c12f7babb7..44f2481a6b7 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -97,7 +97,7 @@ func AnalyzeStrict(statement sqlparser.Statement, currentDb string, si SchemaInf return st, nil } -func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID, fkChecksState sqlparser.FkChecksState) (*SemTable, error) { +func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID, fkChecksState *bool) (*SemTable, error) { var comments *sqlparser.ParsedComments commentedStmt, isCommented := statement.(sqlparser.Commented) if isCommented { @@ -318,8 +318,8 @@ func (a *analyzer) noteQuerySignature(node sqlparser.SQLNode) { } // getInvolvedForeignKeys gets the foreign keys that might require taking care off when executing the given statement. -func (a *analyzer) getInvolvedForeignKeys(statement sqlparser.Statement, fkChecksState sqlparser.FkChecksState) (map[TableSet][]vindexes.ChildFKInfo, map[TableSet][]vindexes.ParentFKInfo, map[string]sqlparser.UpdateExprs, error) { - if fkChecksState == sqlparser.FkChecksOff { +func (a *analyzer) getInvolvedForeignKeys(statement sqlparser.Statement, fkChecksState *bool) (map[TableSet][]vindexes.ChildFKInfo, map[TableSet][]vindexes.ParentFKInfo, map[string]sqlparser.UpdateExprs, error) { + if fkChecksState != nil && !*fkChecksState { return nil, nil, nil, nil } // There are only the DML statements that require any foreign keys handling. diff --git a/go/vt/vtgate/semantics/analyzer_fk_test.go b/go/vt/vtgate/semantics/analyzer_fk_test.go index 6dd587b1c81..771a9b3ebd9 100644 --- a/go/vt/vtgate/semantics/analyzer_fk_test.go +++ b/go/vt/vtgate/semantics/analyzer_fk_test.go @@ -552,7 +552,8 @@ func TestGetInvolvedForeignKeys(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - childFks, parentFks, childFkUpdateExprs, err := tt.analyzer.getInvolvedForeignKeys(tt.stmt, sqlparser.FkChecksOn) + fkState := true + childFks, parentFks, childFkUpdateExprs, err := tt.analyzer.getInvolvedForeignKeys(tt.stmt, &fkState) if tt.expectedErr != "" { require.EqualError(t, err, tt.expectedErr) return diff --git a/go/vt/vtgate/semantics/info_schema.go b/go/vt/vtgate/semantics/info_schema.go index d402f2f7ca5..b2b5d9036f2 100644 --- a/go/vt/vtgate/semantics/info_schema.go +++ b/go/vt/vtgate/semantics/info_schema.go @@ -1718,8 +1718,8 @@ func (i *infoSchemaWithColumns) ForeignKeyMode(keyspace string) (vschemapb.Keysp return i.inner.ForeignKeyMode(keyspace) } -func (i *infoSchemaWithColumns) GetForeignKeyChecksState() sqlparser.FkChecksState { - return sqlparser.FkChecksUnspecified +func (i *infoSchemaWithColumns) GetForeignKeyChecksState() *bool { + return nil } func (i *infoSchemaWithColumns) KeyspaceError(keyspace string) error { diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 3ed49db66ce..f81e5a1a37f 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -151,7 +151,7 @@ type ( ConnCollation() collations.ID // ForeignKeyMode returns the foreign_key flag value ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyMode, error) - GetForeignKeyChecksState() sqlparser.FkChecksState + GetForeignKeyChecksState() *bool KeyspaceError(keyspace string) error } diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index fbc6fd54868..b6e5da05f93 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -109,7 +109,8 @@ type vcursorImpl struct { // fkChecksState stores the state of foreign key checks variable. // This state is meant to be the final fk checks state after consulting the // session state, and the given query's comments for `SET_VAR` optimizer hints. - fkChecksState sqlparser.FkChecksState + // A nil value represents that no foreign_key_checks value was provided. + fkChecksState *bool ignoreMaxMemoryRows bool vschema *vindexes.VSchema vm VSchemaOperator @@ -1089,10 +1090,10 @@ func (vc *vcursorImpl) keyForPlan(ctx context.Context, query string, buf io.Stri _, _ = buf.WriteString(collations.Local().LookupName(vc.collation)) // The plans are going to be different based on the foreign key checks state. So we need to use that value // as part of the plan's hashing key. - fkChecksState := vc.GetForeignKeyChecksState().String() - if fkChecksState != "" { + fkChecksState := vc.GetForeignKeyChecksState() + if fkChecksState != nil { _, _ = buf.WriteString("+fkChecksState:") - _, _ = buf.WriteString(fkChecksState) + _, _ = buf.WriteString(sqlparser.FkChecksStateString(fkChecksState)) } if vc.destination != nil { @@ -1355,12 +1356,12 @@ func (vc *vcursorImpl) CloneForReplicaWarming(ctx context.Context) engine.VCurso } // UpdateForeignKeyChecksState updates the foreign key checks state of the vcursor. -func (vc *vcursorImpl) UpdateForeignKeyChecksState(fkStateFromQuery sqlparser.FkChecksState) { +func (vc *vcursorImpl) UpdateForeignKeyChecksState(fkStateFromQuery *bool) { // Initialize the state to unspecified. - vc.fkChecksState = sqlparser.FkChecksUnspecified + vc.fkChecksState = nil // If the query has a SET_VAR optimizer hint that explicitly sets the foreign key checks state, // we should use that. - if fkStateFromQuery != sqlparser.FkChecksUnspecified { + if fkStateFromQuery != nil { vc.fkChecksState = fkStateFromQuery return } @@ -1369,14 +1370,16 @@ func (vc *vcursorImpl) UpdateForeignKeyChecksState(fkStateFromQuery sqlparser.Fk if isPresent { switch strings.ToLower(fkVal) { case "on", "1": - vc.fkChecksState = sqlparser.FkChecksOn + val := true + vc.fkChecksState = &val case "off", "0": - vc.fkChecksState = sqlparser.FkChecksOff + val := false + vc.fkChecksState = &val } } } // GetForeignKeyChecksState gets the stored foreign key checks state in the vcursor. -func (vc *vcursorImpl) GetForeignKeyChecksState() sqlparser.FkChecksState { +func (vc *vcursorImpl) GetForeignKeyChecksState() *bool { return vc.fkChecksState } diff --git a/go/vt/vtgate/vcursor_impl_test.go b/go/vt/vtgate/vcursor_impl_test.go index 2a462b06a27..d1a1c53cbaa 100644 --- a/go/vt/vtgate/vcursor_impl_test.go +++ b/go/vt/vtgate/vcursor_impl_test.go @@ -258,9 +258,11 @@ func TestSetTarget(t *testing.T) { } func TestKeyForPlan(t *testing.T) { + valTrue := true + valFalse := false type testCase struct { vschema *vindexes.VSchema - fkState sqlparser.FkChecksState + fkState *bool targetString string expectedPlanPrefixKey string } @@ -284,12 +286,12 @@ func TestKeyForPlan(t *testing.T) { }, { vschema: vschemaWith1KS, targetString: "", - fkState: sqlparser.FkChecksOn, + fkState: &valTrue, expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:On+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1@replica", - fkState: sqlparser.FkChecksOff, + fkState: &valFalse, expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+fkChecksState:Off+Query:SELECT 1", }} From 5769be96dc4543d1ba3fee3e9c6174ad9317951a Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 13 Nov 2023 16:55:49 +0530 Subject: [PATCH 17/25] feat: use new function to set the foreign key checks value to prevent having 2 comments and fix test expectations Signed-off-by: Manan Gupta --- .../testdata/foreignkey_cases.json | 20 +- .../testdata/foreignkey_checks_on_cases.json | 329 +++++++++++++++++- go/vt/vtgate/planbuilder/update.go | 3 +- 3 files changed, 331 insertions(+), 21 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 16487437c58..be826f57638 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -857,7 +857,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set m = 2, col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", "Table": "u_tbl2" } ] @@ -944,7 +944,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = :fkc_upd where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = :fkc_upd where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -1015,7 +1015,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1", "Table": "u_tbl1" } ] @@ -1312,7 +1312,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ tbl3 set coly = tbl3.colx + 10 where tbl3.coly = 10", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set coly = tbl3.colx + 10 where tbl3.coly = 10", "Table": "tbl3" } ] @@ -2037,7 +2037,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set col4 = :fkc_upd where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = :fkc_upd where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -2051,7 +2051,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl7 set foo = 100, col7 = baz + 1 + col7 where bar = 42", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl7 set foo = 100, col7 = baz + 1 + col7 where bar = 42", "Table": "u_tbl7" } ] @@ -2127,7 +2127,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -2153,7 +2153,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl1 set cola = u_multicol_tbl1.cola + 3 where id = 3", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl1 set cola = u_multicol_tbl1.cola + 3 where id = 3", "Table": "u_multicol_tbl1" } ] @@ -2233,7 +2233,7 @@ "CompExprCol": 4 } ], - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = :fkc_upd, colb = :fkc_upd1 where (cola, colb) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = :fkc_upd, colb = :fkc_upd1 where (cola, colb) in ::fkc_vals", "Table": "u_multicol_tbl3" }, { @@ -2245,7 +2245,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl2 set cola = 2, colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.id = 7", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl2 set cola = 2, colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.id = 7", "Table": "u_multicol_tbl2" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json index 943c5005c35..16f7b0f2112 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json @@ -656,8 +656,8 @@ "Name": "unsharded_fk_allow", "Sharded": false }, - "FieldQuery": "select col1, col1 from u_tbl1 where 1 != 1", - "Query": "select col1, col1 from u_tbl1 for update", + "FieldQuery": "select col1 from u_tbl1 where 1 != 1", + "Query": "select col1 from u_tbl1 for update", "Table": "u_tbl1" }, { @@ -715,7 +715,7 @@ "OperatorType": "FkCascade", "BvName": "fkc_vals2", "Cols": [ - 1 + 0 ], "Inputs": [ { @@ -791,12 +791,243 @@ { "comment": "update in a table with non-literal value - set null fail due to child update where condition", "query": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1", - "plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints" + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1", + "Instructions": { + "OperatorType": "FKVerify", + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where 1 != 1", + "Query": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where u_tbl2.col1 + 'bar' is not null and u_tbl2.id = 1 and u_tbl1.col1 is null limit 1 lock in share mode", + "Table": "u_tbl1, u_tbl2" + }, + { + "InputName": "PostVerify", + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2, col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where 1 != 1", + "Query": "select col2, col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where u_tbl2.id = 1 for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "NonLiteralUpdateInfo": [ + { + "UpdateExprCol": 2, + "UpdateExprBvName": "fkc_upd", + "CompExprCol": 1 + } + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", + "Table": "u_tbl2" + } + ] + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl1", + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } }, { "comment": "update in a table with non-literal value - with cascade fail as the cascade value is not known", "query": "update u_tbl1 set m = 2, col1 = x + 'bar' where id = 1", - "plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints" + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl1 set m = 2, col1 = x + 'bar' where id = 1", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col1, col1 <=> u_tbl1.x + 'bar', u_tbl1.x + 'bar' from u_tbl1 where 1 != 1", + "Query": "select col1, col1 <=> u_tbl1.x + 'bar', u_tbl1.x + 'bar' from u_tbl1 where id = 1 for update", + "Table": "u_tbl1" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FkCascade", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "NonLiteralUpdateInfo": [ + { + "UpdateExprCol": 2, + "UpdateExprBvName": "fkc_upd", + "CompExprCol": 1 + } + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col2 from u_tbl2 where 1 != 1", + "Query": "select col2 from u_tbl2 where (col2) in ::fkc_vals for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = :fkc_upd where (col2) in ::fkc_vals", + "Table": "u_tbl2" + } + ] + }, + { + "InputName": "CascadeChild-2", + "OperatorType": "FkCascade", + "BvName": "fkc_vals2", + "Cols": [ + 0 + ], + "NonLiteralUpdateInfo": [ + { + "UpdateExprCol": 2, + "UpdateExprBvName": "fkc_upd1", + "CompExprCol": 1 + } + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col9 from u_tbl9 where 1 != 1", + "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (:fkc_upd1 is null or (u_tbl9.col9) not in ((:fkc_upd1))) for update", + "Table": "u_tbl9" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals3", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", + "Table": "u_tbl8" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (:fkc_upd1 is null or (u_tbl9.col9) not in ((:fkc_upd1)))", + "Table": "u_tbl9" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1", + "Table": "u_tbl1" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl1", + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3", + "unsharded_fk_allow.u_tbl8", + "unsharded_fk_allow.u_tbl9" + ] + } }, { "comment": "update in a table with set null, non-literal value on non-foreign key column - allowed", @@ -872,8 +1103,8 @@ "Name": "unsharded_fk_allow", "Sharded": false }, - "FieldQuery": "select col1, col1 from u_tbl1 where 1 != 1", - "Query": "select col1, col1 from u_tbl1 where id = 1 for update", + "FieldQuery": "select col1 from u_tbl1 where 1 != 1", + "Query": "select col1 from u_tbl1 where id = 1 for update", "Table": "u_tbl1" }, { @@ -931,7 +1162,7 @@ "OperatorType": "FkCascade", "BvName": "fkc_vals2", "Cols": [ - 1 + 0 ], "Inputs": [ { @@ -1012,7 +1243,85 @@ { "comment": "update with fk on cross-shard with a where condition on non-literal value - disallowed", "query": "update tbl3 set coly = colx + 10 where coly = 10", - "plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints" + "plan": { + "QueryType": "UPDATE", + "Original": "update tbl3 set coly = colx + 10 where coly = 10", + "Instructions": { + "OperatorType": "FKVerify", + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Limit", + "Count": "1", + "Inputs": [ + { + "OperatorType": "Projection", + "Expressions": [ + "1 as 1" + ], + "Inputs": [ + { + "OperatorType": "Filter", + "Predicate": "tbl1.t1col1 is null", + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "LeftJoin", + "JoinColumnIndexes": "R:0,R:0", + "JoinVars": { + "tbl3_colx": 0 + }, + "TableName": "tbl3_tbl1", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select tbl3.colx from tbl3 where 1 != 1", + "Query": "select tbl3.colx from tbl3 where tbl3.colx + 10 is not null and tbl3.coly = 10 lock in share mode", + "Table": "tbl3" + }, + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "FieldQuery": "select tbl1.t1col1 from tbl1 where 1 != 1", + "Query": "select tbl1.t1col1 from tbl1 where tbl1.t1col1 = :tbl3_colx + 10 lock in share mode", + "Table": "tbl1" + } + ] + } + ] + } + ] + } + ] + }, + { + "InputName": "PostVerify", + "OperatorType": "Update", + "Variant": "Scatter", + "Keyspace": { + "Name": "sharded_fk_allow", + "Sharded": true + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set coly = tbl3.colx + 10 where tbl3.coly = 10", + "Table": "tbl3" + } + ] + }, + "TablesUsed": [ + "sharded_fk_allow.tbl1", + "sharded_fk_allow.tbl3" + ] + } }, { "comment": "update with fk on cross-shard with a where condition", @@ -1297,7 +1606,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where 1 != 1", - "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where (u_tbl4.col4) in ::fkc_vals and :v1 is not null and u_tbl3.col3 is null limit 1 lock in share mode", "Table": "u_tbl3, u_tbl4" }, { diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index dfb841a4d11..4855bff5a94 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -19,6 +19,7 @@ package planbuilder import ( querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/sysvars" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators" @@ -50,7 +51,7 @@ func gen4UpdateStmtPlanner( if ctx.SemTable.HasNonLiteralForeignKeyUpdate(updStmt.Exprs) { // Since we are running the query with foreign key checks off, we have to verify all the foreign keys validity on vtgate. ctx.VerifyAllFKs = true - updStmt.Comments = updStmt.Comments.Prepend("/*+ SET_VAR(foreign_key_checks=OFF) */").Parsed() + updStmt.SetComments(updStmt.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "Off")) } // Remove all the foreign keys that don't require any handling. From 24526a53db31339f70fd7ecac74fff7e94900a69 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 16 Nov 2023 12:03:00 +0530 Subject: [PATCH 18/25] refactor: extract common code Signed-off-by: Manan Gupta --- go/vt/sqlparser/comments.go | 4 ++++ go/vt/vtgate/planbuilder/operators/delete.go | 12 +--------- go/vt/vtgate/planbuilder/operators/update.go | 25 ++++++++++++-------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index e14a7b7be85..4b60be0366e 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -574,6 +574,10 @@ func ForeignKeyChecksState(stmt Statement) *bool { cmt, ok := stmt.(Commented) if ok { fkChecksVal := cmt.GetParsedComments().GetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name) + // If the value of the `foreign_key_checks` optimizer hint is something that doesn't make sense, + // then MySQL just ignores it and treats it like the case, where it is unspecified. We are choosing + // to have the same behaviour here. If the value doesn't match any of the acceptable values, we return nil, + // that signifies that no value was specified. switch strings.ToLower(fkChecksVal) { case "on", "1": fkState := true diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index bdf38a22195..bd38b849e7c 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -20,7 +20,6 @@ import ( "fmt" "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/sysvars" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/planbuilder/operators/ops" @@ -205,16 +204,7 @@ func createFkCascadeOpForDelete(ctx *plancontext.PlanningContext, parentOp ops.O func createFkChildForDelete(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, cols []int) (*FkChild, error) { bvName := ctx.ReservedVars.ReserveVariable(foreignKeyConstraintValues) - // We only reach this code if foreign key checks are either unspecified or on. - // If foreign key checks are explicity turned on, then we should add the set_var parsed comment too - // since underlying MySQL might have foreign_key_checks as off. - // We run with foreign key checks on because the delete might still fail on MySQL due to a child table - // with RESTRICT constraints. - var parsedComments *sqlparser.ParsedComments - fkChecksState := ctx.VSchema.GetForeignKeyChecksState() - if fkChecksState != nil && *fkChecksState { - parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "On").Parsed() - } + parsedComments := getParsedCommentsForFkChecks(ctx) var childStmt sqlparser.Statement switch fk.OnDelete { case sqlparser.Cascade: diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index d43c414eb98..1cb882a9c27 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -481,16 +481,7 @@ func buildChildUpdOpForSetNull( Right: compExpr, } } - // We only reach this code if foreign key checks are either unspecified or on. - // If foreign key checks are explicity turned on, then we should add the set_var parsed comment too - // since underlying MySQL might have foreign_key_checks as off. - // We run with foreign key checks on because the update might still fail on MySQL due to a child table - // with RESTRICT constraints. - var parsedComments *sqlparser.ParsedComments - fkState := ctx.VSchema.GetForeignKeyChecksState() - if fkState != nil && *fkState { - parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "On").Parsed() - } + parsedComments := getParsedCommentsForFkChecks(ctx) childUpdStmt := &sqlparser.Update{ Exprs: childUpdateExprs, Comments: parsedComments, @@ -500,6 +491,20 @@ func buildChildUpdOpForSetNull( return createOpFromStmt(ctx, childUpdStmt, false, "") } +// getParsedCommentsForFkChecks gets the parsed comments to be set on a child query related to foreign_key_checks session variable. +// We only use this function if foreign key checks are either unspecified or on. +// If foreign key checks are explicity turned on, then we should add the set_var parsed comment too +// since underlying MySQL might have foreign_key_checks as off. +// We run with foreign key checks on because the DML might still fail on MySQL due to a child table +// with RESTRICT constraints. +func getParsedCommentsForFkChecks(ctx *plancontext.PlanningContext) (parsedComments *sqlparser.ParsedComments) { + fkState := ctx.VSchema.GetForeignKeyChecksState() + if fkState != nil && *fkState { + parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "On").Parsed() + } + return parsedComments +} + // createFKVerifyOp creates the verify operator for the parent foreign key constraints. func createFKVerifyOp( ctx *plancontext.PlanningContext, From 2d0f11a2c48ce97ed3501586aec2ccad19f3b3f8 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 16 Nov 2023 13:19:53 +0530 Subject: [PATCH 19/25] test: add failing testcase Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index a05f0de7ac1..654f9b282b7 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -881,6 +881,14 @@ func TestFkQueries(t *testing.T) { "update fk_multicol_t15 set cola = 3, colb = (id * 2) - 2", }, }, + { + name: "Update a child table which doesn't cause an update, but parent doesn't have that value", + queries: []string{ + "insert into fk_t10 (id, col) values (1,1),(2,2)", + "insert /*+ SET_VAR(foreign_key_checks=0) */ into fk_t11 (id, col) values (1,1),(2,2),(5,5)", + "update fk_t11 set col = id where id in (1, 5)", + }, + }, } for _, testcase := range testcases { From 495e2c3a873f59d93c45d55e936250d2f4e81de4 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 16 Nov 2023 13:49:28 +0530 Subject: [PATCH 20/25] feat: fix verify selection query to discount rows that aren't changing Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 14 ++++++++++++++ go/vt/vtgate/planbuilder/operators/update.go | 18 ++++++++++++++++-- go/vt/vtgate/semantics/early_rewriter.go | 5 +++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index 654f9b282b7..08dd5660e67 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "strings" "testing" "time" @@ -889,6 +890,14 @@ func TestFkQueries(t *testing.T) { "update fk_t11 set col = id where id in (1, 5)", }, }, + { + name: "Update a child table from a null to a value that parent doesn't have", + queries: []string{ + "insert into fk_t10 (id, col) values (1,1),(2,2)", + "insert into fk_t11 (id, col) values (1,1),(2,2),(5,NULL)", + "update fk_t11 set col = id where id in (1, 5)", + }, + }, } for _, testcase := range testcases { @@ -955,6 +964,11 @@ func TestFkOneCase(t *testing.T) { ensureDatabaseState(t, mcmp.MySQLConn, true) for _, query := range queries { + if strings.HasPrefix(query, "vexplain") { + res := utils.Exec(t, mcmp.VtConn, query) + log.Errorf("Query %v, Result - %v", query, res.Rows) + continue + } _, _ = mcmp.ExecAllowAndCompareError(query) if t.Failed() { log.Errorf("Query failed - %v", query) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 1cb882a9c27..c8657190507 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -549,14 +549,14 @@ func createFKVerifyOp( // Each parent foreign key constraint is verified by an anti join query of the form: // select 1 from child_tbl left join parent_tbl on -// where and and and limit 1 +// where and and and and limit 1 // E.g: // Child (c1, c2) references Parent (p1, p2) // update Child set c1 = c2 + 1 where id = 1 // verify query: // select 1 from Child left join Parent on Parent.p1 = Child.c2 + 1 and Parent.p2 = Child.c2 // where Parent.p1 is null and Parent.p2 is null and Child.id = 1 and Child.c2 + 1 is not null -// and Child.c2 is not null +// and Child.c2 is not null and not ((Child.c1) <=> (Child.c2 + 1)) // limit 1 func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, pFK vindexes.ParentFKInfo) (ops.Operator, error) { childTblExpr := updStmt.TableExprs[0].(*sqlparser.AliasedTableExpr) @@ -567,6 +567,8 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, updS parentTbl := pFK.Table.GetTableName() var whereCond sqlparser.Expr var joinCond sqlparser.Expr + var notEqualColNames sqlparser.ValTuple + var notEqualExprs sqlparser.ValTuple for idx, column := range pFK.ChildColumns { var matchedExpr *sqlparser.UpdateExpr for _, updateExpr := range updStmt.Exprs { @@ -595,7 +597,9 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, updS Right: sqlparser.NewColNameWithQualifier(pFK.ChildColumns[idx].String(), childTbl), } } else { + notEqualColNames = append(notEqualColNames, prefixColNames(childTbl, matchedExpr.Name)) prefixedMatchExpr := prefixColNames(childTbl, matchedExpr.Expr) + notEqualExprs = append(notEqualExprs, prefixedMatchExpr) joinExpr = &sqlparser.ComparisonExpr{ Operator: sqlparser.EqualOp, Left: sqlparser.NewColNameWithQualifier(pFK.ParentColumns[idx].String(), parentTbl), @@ -617,6 +621,16 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, updS joinCond = &sqlparser.AndExpr{Left: joinCond, Right: joinExpr} whereCond = &sqlparser.AndExpr{Left: whereCond, Right: predicate} } + whereCond = &sqlparser.AndExpr{ + Left: whereCond, + Right: &sqlparser.NotExpr{ + Expr: &sqlparser.ComparisonExpr{ + Operator: sqlparser.NullSafeEqualOp, + Left: notEqualColNames, + Right: notEqualExprs, + }, + }, + } // add existing where condition on the update statement if updStmt.Where != nil { whereCond = &sqlparser.AndExpr{Left: whereCond, Right: prefixColNames(childTbl, updStmt.Where.Expr)} diff --git a/go/vt/vtgate/semantics/early_rewriter.go b/go/vt/vtgate/semantics/early_rewriter.go index 0349ef3f79d..1a957c8ae60 100644 --- a/go/vt/vtgate/semantics/early_rewriter.go +++ b/go/vt/vtgate/semantics/early_rewriter.go @@ -107,6 +107,11 @@ func rewriteNotExpr(cursor *sqlparser.Cursor, node *sqlparser.NotExpr) { return } + // There is no inverse operator for NullSafeEqualOp. + // There doesn't exist a null safe non-equality. + if cmp.Operator == sqlparser.NullSafeEqualOp { + return + } cmp.Operator = sqlparser.Inverse(cmp.Operator) cursor.Replace(cmp) } From 4624a2e9790acd8bb100f3891efbdb3ca378c842 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 16 Nov 2023 14:55:48 +0530 Subject: [PATCH 21/25] test: update test expectations Signed-off-by: Manan Gupta --- .../testdata/foreignkey_cases.json | 44 ++++++++--------- .../testdata/foreignkey_checks_on_cases.json | 47 ++++++++++--------- 2 files changed, 47 insertions(+), 44 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index d55fccd8fbe..50b4d518b2d 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -527,7 +527,7 @@ "Sharded": true }, "FieldQuery": "select 1 from tbl10 where 1 != 1", - "Query": "select 1 from tbl10 lock in share mode", + "Query": "select 1 from tbl10 where not (tbl10.col) <=> ('foo') lock in share mode", "Table": "tbl10" }, { @@ -558,7 +558,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update tbl10 set col = 'foo'", + "Query": "update tbl10 set tbl10.col = 'foo'", "Table": "tbl10" } ] @@ -806,7 +806,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where 1 != 1", - "Query": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where u_tbl2.col1 + 'bar' is not null and u_tbl2.id = 1 and u_tbl1.col1 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where u_tbl2.col1 + 'bar' is not null and not (u_tbl2.col2) <=> (u_tbl2.col1 + 'bar') and u_tbl2.id = 1 and u_tbl1.col1 is null limit 1 lock in share mode", "Table": "u_tbl1, u_tbl2" }, { @@ -821,8 +821,8 @@ "Name": "unsharded_fk_allow", "Sharded": false }, - "FieldQuery": "select col2, col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where 1 != 1", - "Query": "select col2, col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where u_tbl2.id = 1 for update", + "FieldQuery": "select col2, u_tbl2.col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where 1 != 1", + "Query": "select col2, u_tbl2.col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where u_tbl2.id = 1 for update", "Table": "u_tbl2" }, { @@ -858,7 +858,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, u_tbl2.col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", "Table": "u_tbl2" } ] @@ -1284,7 +1284,7 @@ "Sharded": true }, "FieldQuery": "select tbl3.colx from tbl3 where 1 != 1", - "Query": "select tbl3.colx from tbl3 where tbl3.colx + 10 is not null and tbl3.coly = 10 lock in share mode", + "Query": "select tbl3.colx from tbl3 where tbl3.colx + 10 is not null and not (tbl3.coly) <=> (tbl3.colx + 10) and tbl3.coly = 10 lock in share mode", "Table": "tbl3" }, { @@ -1315,7 +1315,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set coly = tbl3.colx + 10 where tbl3.coly = 10", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set tbl3.coly = tbl3.colx + 10 where tbl3.coly = 10", "Table": "tbl3" } ] @@ -1364,7 +1364,7 @@ "Sharded": true }, "FieldQuery": "select 1 from tbl3 where 1 != 1", - "Query": "select 1 from tbl3 where tbl3.coly = 10 lock in share mode", + "Query": "select 1 from tbl3 where not (tbl3.coly) <=> (20) and tbl3.coly = 10 lock in share mode", "Table": "tbl3" }, { @@ -1395,7 +1395,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update tbl3 set coly = 20 where tbl3.coly = 10", + "Query": "update tbl3 set tbl3.coly = 20 where tbl3.coly = 10", "Table": "tbl3" } ] @@ -1444,7 +1444,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where 1 != 1", - "Query": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where (u_tbl8.col8) in ::fkc_vals and u_tbl9.col9 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where not (u_tbl8.col8) <=> ('foo') and (u_tbl8.col8) in ::fkc_vals and u_tbl9.col9 is null limit 1 lock in share mode", "Table": "u_tbl8, u_tbl9" }, { @@ -1456,7 +1456,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set u_tbl8.col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", "Table": "u_tbl8" } ] @@ -1520,7 +1520,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where 1 != 1", - "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where not (u_tbl4.col4) <=> ('foo') and (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", "Table": "u_tbl3, u_tbl4" }, { @@ -1544,7 +1544,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1609,7 +1609,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where 1 != 1", - "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where (u_tbl4.col4) in ::fkc_vals and :v1 is not null and u_tbl3.col3 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where not (u_tbl4.col4) <=> (:v1) and (u_tbl4.col4) in ::fkc_vals and :v1 is not null and u_tbl3.col3 is null limit 1 lock in share mode", "Table": "u_tbl3, u_tbl4" }, { @@ -1633,7 +1633,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -2121,7 +2121,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :fkc_upd where 1 != 1", - "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :fkc_upd where (u_tbl4.col4) in ::fkc_vals and :fkc_upd is not null and u_tbl3.col3 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :fkc_upd where not (u_tbl4.col4) <=> (:fkc_upd) and (u_tbl4.col4) in ::fkc_vals and :fkc_upd is not null and u_tbl3.col3 is null limit 1 lock in share mode", "Table": "u_tbl3, u_tbl4" }, { @@ -2145,7 +2145,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = :fkc_upd where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = :fkc_upd where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -2297,7 +2297,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_multicol_tbl2 left join u_multicol_tbl1 on u_multicol_tbl1.cola = 2 and u_multicol_tbl1.colb = u_multicol_tbl2.colc - 2 where 1 != 1", - "Query": "select 1 from u_multicol_tbl2 left join u_multicol_tbl1 on u_multicol_tbl1.cola = 2 and u_multicol_tbl1.colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.colc - 2 is not null and u_multicol_tbl2.id = 7 and u_multicol_tbl1.cola is null and u_multicol_tbl1.colb is null limit 1 lock in share mode", + "Query": "select 1 from u_multicol_tbl2 left join u_multicol_tbl1 on u_multicol_tbl1.cola = 2 and u_multicol_tbl1.colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.colc - 2 is not null and not (u_multicol_tbl2.cola, u_multicol_tbl2.colb) <=> (2, u_multicol_tbl2.colc - 2) and u_multicol_tbl2.id = 7 and u_multicol_tbl1.cola is null and u_multicol_tbl1.colb is null limit 1 lock in share mode", "Table": "u_multicol_tbl1, u_multicol_tbl2" }, { @@ -2312,8 +2312,8 @@ "Name": "unsharded_fk_allow", "Sharded": false }, - "FieldQuery": "select cola, colb, cola <=> 2, 2, colb <=> u_multicol_tbl2.colc - 2, u_multicol_tbl2.colc - 2 from u_multicol_tbl2 where 1 != 1", - "Query": "select cola, colb, cola <=> 2, 2, colb <=> u_multicol_tbl2.colc - 2, u_multicol_tbl2.colc - 2 from u_multicol_tbl2 where u_multicol_tbl2.id = 7 for update", + "FieldQuery": "select cola, colb, u_multicol_tbl2.cola <=> 2, 2, u_multicol_tbl2.colb <=> u_multicol_tbl2.colc - 2, u_multicol_tbl2.colc - 2 from u_multicol_tbl2 where 1 != 1", + "Query": "select cola, colb, u_multicol_tbl2.cola <=> 2, 2, u_multicol_tbl2.colb <=> u_multicol_tbl2.colc - 2, u_multicol_tbl2.colc - 2 from u_multicol_tbl2 where u_multicol_tbl2.id = 7 for update", "Table": "u_multicol_tbl2" }, { @@ -2356,7 +2356,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl2 set cola = 2, colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.id = 7", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl2 set u_multicol_tbl2.cola = 2, u_multicol_tbl2.colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.id = 7", "Table": "u_multicol_tbl2" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json index 7a7af2d12ec..3cfcb263e2e 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json @@ -527,7 +527,7 @@ "Sharded": true }, "FieldQuery": "select 1 from tbl10 where 1 != 1", - "Query": "select 1 from tbl10 lock in share mode", + "Query": "select 1 from tbl10 where not (tbl10.col) <=> ('foo') lock in share mode", "Table": "tbl10" }, { @@ -558,7 +558,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl10 set col = 'foo'", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl10 set tbl10.col = 'foo'", "Table": "tbl10" } ] @@ -806,7 +806,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where 1 != 1", - "Query": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where u_tbl2.col1 + 'bar' is not null and u_tbl2.id = 1 and u_tbl1.col1 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where u_tbl2.col1 + 'bar' is not null and not (u_tbl2.col2) <=> (u_tbl2.col1 + 'bar') and u_tbl2.id = 1 and u_tbl1.col1 is null limit 1 lock in share mode", "Table": "u_tbl1, u_tbl2" }, { @@ -821,8 +821,8 @@ "Name": "unsharded_fk_allow", "Sharded": false }, - "FieldQuery": "select col2, col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where 1 != 1", - "Query": "select col2, col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where u_tbl2.id = 1 for update", + "FieldQuery": "select col2, u_tbl2.col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where 1 != 1", + "Query": "select col2, u_tbl2.col2 <=> u_tbl2.col1 + 'bar', u_tbl2.col1 + 'bar' from u_tbl2 where u_tbl2.id = 1 for update", "Table": "u_tbl2" }, { @@ -840,9 +840,10 @@ ], "NonLiteralUpdateInfo": [ { + "ExprCol": 0, + "CompExprCol": 1, "UpdateExprCol": 2, - "UpdateExprBvName": "fkc_upd", - "CompExprCol": 1 + "UpdateExprBvName": "fkc_upd" } ], "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))", @@ -857,7 +858,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, u_tbl2.col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", "Table": "u_tbl2" } ] @@ -901,9 +902,10 @@ ], "NonLiteralUpdateInfo": [ { + "ExprCol": 0, + "CompExprCol": 1, "UpdateExprCol": 2, - "UpdateExprBvName": "fkc_upd", - "CompExprCol": 1 + "UpdateExprBvName": "fkc_upd" } ], "Inputs": [ @@ -958,9 +960,10 @@ ], "NonLiteralUpdateInfo": [ { + "ExprCol": 0, + "CompExprCol": 1, "UpdateExprCol": 2, - "UpdateExprBvName": "fkc_upd1", - "CompExprCol": 1 + "UpdateExprBvName": "fkc_upd1" } ], "Inputs": [ @@ -1281,7 +1284,7 @@ "Sharded": true }, "FieldQuery": "select tbl3.colx from tbl3 where 1 != 1", - "Query": "select tbl3.colx from tbl3 where tbl3.colx + 10 is not null and tbl3.coly = 10 lock in share mode", + "Query": "select tbl3.colx from tbl3 where tbl3.colx + 10 is not null and not (tbl3.coly) <=> (tbl3.colx + 10) and tbl3.coly = 10 lock in share mode", "Table": "tbl3" }, { @@ -1312,7 +1315,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set coly = tbl3.colx + 10 where tbl3.coly = 10", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set tbl3.coly = tbl3.colx + 10 where tbl3.coly = 10", "Table": "tbl3" } ] @@ -1361,7 +1364,7 @@ "Sharded": true }, "FieldQuery": "select 1 from tbl3 where 1 != 1", - "Query": "select 1 from tbl3 where tbl3.coly = 10 lock in share mode", + "Query": "select 1 from tbl3 where not (tbl3.coly) <=> (20) and tbl3.coly = 10 lock in share mode", "Table": "tbl3" }, { @@ -1392,7 +1395,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl3 set coly = 20 where tbl3.coly = 10", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl3 set tbl3.coly = 20 where tbl3.coly = 10", "Table": "tbl3" } ] @@ -1441,7 +1444,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where 1 != 1", - "Query": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where (u_tbl8.col8) in ::fkc_vals and u_tbl9.col9 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl8 left join u_tbl9 on u_tbl9.col9 = 'foo' where not (u_tbl8.col8) <=> ('foo') and (u_tbl8.col8) in ::fkc_vals and u_tbl9.col9 is null limit 1 lock in share mode", "Table": "u_tbl8, u_tbl9" }, { @@ -1453,7 +1456,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set u_tbl8.col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", "Table": "u_tbl8" } ] @@ -1517,7 +1520,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where 1 != 1", - "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = 'foo' where not (u_tbl4.col4) <=> ('foo') and (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", "Table": "u_tbl3, u_tbl4" }, { @@ -1541,7 +1544,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1606,7 +1609,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where 1 != 1", - "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where (u_tbl4.col4) in ::fkc_vals and :v1 is not null and u_tbl3.col3 is null limit 1 lock in share mode", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where not (u_tbl4.col4) <=> (:v1) and (u_tbl4.col4) in ::fkc_vals and :v1 is not null and u_tbl3.col3 is null limit 1 lock in share mode", "Table": "u_tbl3, u_tbl4" }, { @@ -1630,7 +1633,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] From 06d6ca900993badca313135392bea20746a57436 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Fri, 17 Nov 2023 06:28:56 +0530 Subject: [PATCH 22/25] feat: simplify comment parsing Signed-off-by: Manan Gupta --- go/vt/sqlparser/comments.go | 59 ++++++++++++++----------------------- 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 4b60be0366e..fbc1e17ba2f 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -425,48 +425,21 @@ func getOptimizerHint(initialPos int, commentStr string) (pos int, ohNameStart i // skipUntilParenthesisEnd reads the comment string given the initial position and skips over until // it has seen the end of opening bracket. func skipUntilParenthesisEnd(pos int, commentStr string) int { - // This is a problem of bracket matching. We solve this using a stack, because strings are also part of the comment - // that can also have '(' and ')' characters. So we can't just skip over until we see a closing parenthesis. - // Initialize a stack with opening bracket. - stack := []byte{'('} for pos < len(commentStr) { switch commentStr[pos] { - case '(': - // If we see an opening bracket, we put it into the stack - // only if we aren't currently reading a string. - if stack[len(stack)-1] == '(' { - stack = append(stack, '(') - } case ')': - // If we see a closing bracket, we try to remove an opening - // bracket from the stack. - if stack[len(stack)-1] == '(' { - stack = stack[:len(stack)-1] - } + // If we see a closing bracket, we have found the ending of our parenthesis. + return pos case '\'': - // If we see a single quote character, then it can either - // signify the ending of a previously started string, it could be - // part of a string that is encased in double quotes, or it could - // be the start of a new string. - if stack[len(stack)-1] == '\'' { - stack = stack[:len(stack)-1] - } else if stack[len(stack)-1] != '"' { - stack = append(stack, '\'') - } + // If we see a single quote character, then it signifies the start of a new string. + // We wait until we see the end of this string. + pos++ + pos = skipUntilCharacter(pos, commentStr, '\'') case '"': - // If we see a double quote character, then it can either - // signify the ending of a previously started string, it could be - // part of a string that is encased in single quotes, or it could - // be the start of a new string. - if stack[len(stack)-1] == '"' { - stack = stack[:len(stack)-1] - } else if stack[len(stack)-1] != '\'' { - stack = append(stack, '"') - } - } - // The first time the stack becomes empty, signifies the ending of the parenthesis we set out to match. - if len(stack) == 0 { - return pos + // If we see a double quote character, then it signifies the start of a new string. + // We wait until we see the end of this string. + pos++ + pos = skipUntilCharacter(pos, commentStr, '"') } pos++ } @@ -474,6 +447,18 @@ func skipUntilParenthesisEnd(pos int, commentStr string) int { return pos } +// skipUntilCharacter skips until the given character has been seen in the comment string, given the starting position. +func skipUntilCharacter(pos int, commentStr string, ch byte) int { + for pos < len(commentStr) { + if commentStr[pos] != ch { + pos++ + continue + } + break + } + return pos +} + // skipBlanks skips over space characters from the comment string, given the starting position. func skipBlanks(pos int, commentStr string) int { for pos < len(commentStr) { From c3230cffb233754305bb6342eea2a2e87b5cf5bf Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 20 Nov 2023 11:11:45 +0530 Subject: [PATCH 23/25] refactor: change prefix used in plan key Signed-off-by: Manan Gupta --- go/vt/vtgate/vcursor_impl.go | 2 +- go/vt/vtgate/vcursor_impl_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index b6e5da05f93..a6151e8cb99 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -1092,7 +1092,7 @@ func (vc *vcursorImpl) keyForPlan(ctx context.Context, query string, buf io.Stri // as part of the plan's hashing key. fkChecksState := vc.GetForeignKeyChecksState() if fkChecksState != nil { - _, _ = buf.WriteString("+fkChecksState:") + _, _ = buf.WriteString("+fkc:") _, _ = buf.WriteString(sqlparser.FkChecksStateString(fkChecksState)) } diff --git a/go/vt/vtgate/vcursor_impl_test.go b/go/vt/vtgate/vcursor_impl_test.go index d1a1c53cbaa..e20e297c73a 100644 --- a/go/vt/vtgate/vcursor_impl_test.go +++ b/go/vt/vtgate/vcursor_impl_test.go @@ -287,12 +287,12 @@ func TestKeyForPlan(t *testing.T) { vschema: vschemaWith1KS, targetString: "", fkState: &valTrue, - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkChecksState:On+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkc:On+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1@replica", fkState: &valFalse, - expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+fkChecksState:Off+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+fkc:Off+Query:SELECT 1", }} for i, tc := range tests { From 12d409b6b0af4ed50a715bc3bd20f247b7b36a4e Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 20 Nov 2023 11:18:43 +0530 Subject: [PATCH 24/25] refactor: rename Off to OFF and On to ON Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/update.go | 4 +- .../testdata/foreignkey_cases.json | 34 ++++----- .../testdata/foreignkey_checks_off_cases.json | 6 +- .../testdata/foreignkey_checks_on_cases.json | 70 +++++++++---------- go/vt/vtgate/planbuilder/update.go | 2 +- 5 files changed, 58 insertions(+), 58 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 9515d0e388f..1f122a950d4 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -438,7 +438,7 @@ func buildChildUpdOpForCascade(ctx *plancontext.PlanningContext, fk vindexes.Chi // Because we could be updating the child to a non-null value, // We have to run with foreign key checks OFF because the parent isn't guaranteed to have // the data being updated to. - parsedComments := (&sqlparser.ParsedComments{}).SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "Off").Parsed() + parsedComments := (&sqlparser.ParsedComments{}).SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "OFF").Parsed() childUpdStmt := &sqlparser.Update{ Comments: parsedComments, Exprs: childUpdateExprs, @@ -508,7 +508,7 @@ func buildChildUpdOpForSetNull( func getParsedCommentsForFkChecks(ctx *plancontext.PlanningContext) (parsedComments *sqlparser.ParsedComments) { fkState := ctx.VSchema.GetForeignKeyChecksState() if fkState != nil && *fkState { - parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "On").Parsed() + parsedComments = parsedComments.SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "ON").Parsed() } return parsedComments } diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 50b4d518b2d..99679d698a2 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -705,7 +705,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -858,7 +858,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, u_tbl2.col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set m = 2, u_tbl2.col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", "Table": "u_tbl2" } ] @@ -946,7 +946,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = :fkc_upd where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = :fkc_upd where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -1018,7 +1018,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1", "Table": "u_tbl1" } ] @@ -1155,7 +1155,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 2 where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = 2 where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -1315,7 +1315,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set tbl3.coly = tbl3.colx + 10 where tbl3.coly = 10", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ tbl3 set tbl3.coly = tbl3.colx + 10 where tbl3.coly = 10", "Table": "tbl3" } ] @@ -1456,7 +1456,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set u_tbl8.col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl8 set u_tbl8.col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", "Table": "u_tbl8" } ] @@ -1544,7 +1544,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set u_tbl4.col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1633,7 +1633,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set u_tbl4.col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1857,7 +1857,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -1951,7 +1951,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -2145,7 +2145,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = :fkc_upd where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set u_tbl4.col4 = :fkc_upd where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -2159,7 +2159,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl7 set foo = 100, col7 = baz + 1 + col7 where bar = 42", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl7 set foo = 100, col7 = baz + 1 + col7 where bar = 42", "Table": "u_tbl7" } ] @@ -2236,7 +2236,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -2262,7 +2262,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl1 set cola = u_multicol_tbl1.cola + 3 where id = 3", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl1 set cola = u_multicol_tbl1.cola + 3 where id = 3", "Table": "u_multicol_tbl1" } ] @@ -2344,7 +2344,7 @@ "UpdateExprBvName": "fkc_upd1" } ], - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = :fkc_upd, colb = :fkc_upd1 where (cola, colb) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = :fkc_upd, colb = :fkc_upd1 where (cola, colb) in ::fkc_vals", "Table": "u_multicol_tbl3" }, { @@ -2356,7 +2356,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl2 set u_multicol_tbl2.cola = 2, u_multicol_tbl2.colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.id = 7", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl2 set u_multicol_tbl2.cola = 2, u_multicol_tbl2.colb = u_multicol_tbl2.colc - 2 where u_multicol_tbl2.id = 7", "Table": "u_multicol_tbl2" } ] diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json index 6674ae6a346..968fb8679fb 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_off_cases.json @@ -137,7 +137,7 @@ 3, 4 ], - "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from multicol_tbl2 where (colb, cola, x, colc, y) in ::fkc_vals", + "Query": "delete /*+ SET_VAR(foreign_key_checks=ON) */ from multicol_tbl2 where (colb, cola, x, colc, y) in ::fkc_vals", "Table": "multicol_tbl2" }, { @@ -274,7 +274,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (tbl4.t4col4) not in (('foo'))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (tbl4.t4col4) not in (('foo'))", "Table": "tbl4" }, { @@ -386,7 +386,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set col_ref = null where (col_ref) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ tbl4 set col_ref = null where (col_ref) in ::fkc_vals", "Table": "tbl4" }, { diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json index 3cfcb263e2e..862c18b068d 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json @@ -108,7 +108,7 @@ 3, 4 ], - "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from multicol_tbl2 where (colb, cola, x, colc, y) in ::fkc_vals", + "Query": "delete /*+ SET_VAR(foreign_key_checks=ON) */ from multicol_tbl2 where (colb, cola, x, colc, y) in ::fkc_vals", "Table": "multicol_tbl2" }, { @@ -171,7 +171,7 @@ "Cols": [ 0 ], - "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (col4) in ::fkc_vals", + "Query": "delete /*+ SET_VAR(foreign_key_checks=ON) */ from tbl4 where (col4) in ::fkc_vals", "Table": "tbl4" }, { @@ -187,7 +187,7 @@ "Cols": [ 1 ], - "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (t4col4) in ::fkc_vals1", + "Query": "delete /*+ SET_VAR(foreign_key_checks=ON) */ from tbl4 where (t4col4) in ::fkc_vals1", "Table": "tbl4" }, { @@ -249,7 +249,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals", "Table": "u_tbl8" }, { @@ -328,7 +328,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in (('bar'))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in (('bar'))", "Table": "u_tbl3" }, { @@ -439,7 +439,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (tbl4.t4col4) not in (('foo'))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (tbl4.t4col4) not in (('foo'))", "Table": "tbl4" }, { @@ -612,7 +612,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ tbl4 set col_ref = null where (col_ref) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ tbl4 set col_ref = null where (col_ref) in ::fkc_vals", "Table": "tbl4" }, { @@ -693,7 +693,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in (('foo'))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in (('foo'))", "Table": "u_tbl3" }, { @@ -705,7 +705,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = 'foo' where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -743,7 +743,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", "Table": "u_tbl8" }, { @@ -755,7 +755,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in (('foo'))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in (('foo'))", "Table": "u_tbl9" } ] @@ -846,7 +846,7 @@ "UpdateExprBvName": "fkc_upd" } ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))", "Table": "u_tbl3" }, { @@ -858,7 +858,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set m = 2, u_tbl2.col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set m = 2, u_tbl2.col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1", "Table": "u_tbl2" } ] @@ -934,7 +934,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))", "Table": "u_tbl3" }, { @@ -946,7 +946,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = :fkc_upd where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = :fkc_upd where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -992,7 +992,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", "Table": "u_tbl8" }, { @@ -1004,7 +1004,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (:fkc_upd1 is null or (u_tbl9.col9) not in ((:fkc_upd1)))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (:fkc_upd1 is null or (u_tbl9.col9) not in ((:fkc_upd1)))", "Table": "u_tbl9" } ] @@ -1018,7 +1018,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1", "Table": "u_tbl1" } ] @@ -1066,7 +1066,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in ((2))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in ((2))", "Table": "u_tbl3" }, { @@ -1143,7 +1143,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in ((2))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in ((2))", "Table": "u_tbl3" }, { @@ -1155,7 +1155,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl2 set col2 = 2 where (col2) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = 2 where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -1193,7 +1193,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl8 set col8 = null where (col8) in ::fkc_vals3", "Table": "u_tbl8" }, { @@ -1205,7 +1205,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in ((2))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in ((2))", "Table": "u_tbl9" } ] @@ -1315,7 +1315,7 @@ "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ tbl3 set tbl3.coly = tbl3.colx + 10 where tbl3.coly = 10", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ tbl3 set tbl3.coly = tbl3.colx + 10 where tbl3.coly = 10", "Table": "tbl3" } ] @@ -1456,7 +1456,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl8 set u_tbl8.col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl8 set u_tbl8.col8 = 'foo' where (u_tbl8.col8) in ::fkc_vals", "Table": "u_tbl8" } ] @@ -1544,7 +1544,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set u_tbl4.col4 = 'foo' where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1633,7 +1633,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_tbl4 set u_tbl4.col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set u_tbl4.col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", "Table": "u_tbl4" } ] @@ -1749,7 +1749,7 @@ "Cols": [ 0 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals1", "Table": "u_tbl3" }, { @@ -1761,7 +1761,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from u_tbl2 where (col2) in ::fkc_vals", + "Query": "delete /*+ SET_VAR(foreign_key_checks=ON) */ from u_tbl2 where (col2) in ::fkc_vals", "Table": "u_tbl2" } ] @@ -1857,7 +1857,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -1869,7 +1869,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((1, 2))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((1, 2))", "Table": "u_multicol_tbl2" } ] @@ -1951,7 +1951,7 @@ 0, 1 ], - "Query": "update /*+ SET_VAR(foreign_key_checks=Off) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", "Table": "u_multicol_tbl3" }, { @@ -1963,7 +1963,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((:v1, :v2))))", + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((:v1, :v2))))", "Table": "u_multicol_tbl2" } ] @@ -2029,7 +2029,7 @@ "Cols": [ 0 ], - "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (col4) in ::fkc_vals", + "Query": "delete /*+ SET_VAR(foreign_key_checks=ON) */ from tbl4 where (col4) in ::fkc_vals", "Table": "tbl4" }, { @@ -2045,7 +2045,7 @@ "Cols": [ 1 ], - "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from tbl4 where (t4col4) in ::fkc_vals1", + "Query": "delete /*+ SET_VAR(foreign_key_checks=ON) */ from tbl4 where (t4col4) in ::fkc_vals1", "Table": "tbl4" }, { diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index 4855bff5a94..9bcc8691f8e 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -51,7 +51,7 @@ func gen4UpdateStmtPlanner( if ctx.SemTable.HasNonLiteralForeignKeyUpdate(updStmt.Exprs) { // Since we are running the query with foreign key checks off, we have to verify all the foreign keys validity on vtgate. ctx.VerifyAllFKs = true - updStmt.SetComments(updStmt.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "Off")) + updStmt.SetComments(updStmt.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, "OFF")) } // Remove all the foreign keys that don't require any handling. From f55e95b6a939ad03baf5425932d8097471d22b07 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 20 Nov 2023 12:42:48 +0530 Subject: [PATCH 25/25] feat: remove fk state in plan key Signed-off-by: Manan Gupta --- go/vt/vtgate/vcursor_impl.go | 7 ------- go/vt/vtgate/vcursor_impl_test.go | 10 ++-------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index a6151e8cb99..9ec4bb0dc03 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -1088,13 +1088,6 @@ func (vc *vcursorImpl) keyForPlan(ctx context.Context, query string, buf io.Stri _, _ = buf.WriteString(vindexes.TabletTypeSuffix[vc.tabletType]) _, _ = buf.WriteString("+Collate:") _, _ = buf.WriteString(collations.Local().LookupName(vc.collation)) - // The plans are going to be different based on the foreign key checks state. So we need to use that value - // as part of the plan's hashing key. - fkChecksState := vc.GetForeignKeyChecksState() - if fkChecksState != nil { - _, _ = buf.WriteString("+fkc:") - _, _ = buf.WriteString(sqlparser.FkChecksStateString(fkChecksState)) - } if vc.destination != nil { switch vc.destination.(type) { diff --git a/go/vt/vtgate/vcursor_impl_test.go b/go/vt/vtgate/vcursor_impl_test.go index e20e297c73a..77be183eacd 100644 --- a/go/vt/vtgate/vcursor_impl_test.go +++ b/go/vt/vtgate/vcursor_impl_test.go @@ -258,11 +258,8 @@ func TestSetTarget(t *testing.T) { } func TestKeyForPlan(t *testing.T) { - valTrue := true - valFalse := false type testCase struct { vschema *vindexes.VSchema - fkState *bool targetString string expectedPlanPrefixKey string } @@ -286,13 +283,11 @@ func TestKeyForPlan(t *testing.T) { }, { vschema: vschemaWith1KS, targetString: "", - fkState: &valTrue, - expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+fkc:On+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@primary+Collate:utf8mb4_0900_ai_ci+Query:SELECT 1", }, { vschema: vschemaWith1KS, targetString: "ks1@replica", - fkState: &valFalse, - expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+fkc:Off+Query:SELECT 1", + expectedPlanPrefixKey: "ks1@replica+Collate:utf8mb4_0900_ai_ci+Query:SELECT 1", }} for i, tc := range tests { @@ -302,7 +297,6 @@ func TestKeyForPlan(t *testing.T) { vc, err := newVCursorImpl(ss, sqlparser.MarginComments{}, nil, nil, &fakeVSchemaOperator{vschema: tc.vschema}, tc.vschema, srvtopo.NewResolver(&fakeTopoServer{}, nil, ""), nil, false, querypb.ExecuteOptions_Gen4) require.NoError(t, err) vc.vschema = tc.vschema - vc.fkChecksState = tc.fkState var buf strings.Builder vc.keyForPlan(context.Background(), "SELECT 1", &buf)