Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Foreign_key_checks a Vitess Aware variable #14484

Merged
merged 29 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2809eda
feat: make foreign_key_checks system aware vitess variable
GuptaManan100 Nov 6, 2023
185b4ea
test: add test for foreign-key-checks being marked as required in nee…
GuptaManan100 Nov 6, 2023
39e44ac
feat: add function for getting mysql set var value for the given key
GuptaManan100 Nov 6, 2023
ecdf594
feat: add function to set the foreign key checks on the vtgate session
GuptaManan100 Nov 6, 2023
a4eda3f
feat: add function to set a variable in SET_VAR optimizer hint
GuptaManan100 Nov 6, 2023
cc274a7
feat: read foreign_key_checks from comments and rewrite to appropriat…
GuptaManan100 Nov 6, 2023
d8a6a04
feat: add fkChecksState to the plan key
GuptaManan100 Nov 6, 2023
48e875c
feat: ignore foreign keys planning if fk checks are turned off
GuptaManan100 Nov 7, 2023
5966665
feat: add tests and fix minor bugs
GuptaManan100 Nov 9, 2023
48c169e
test: add tests for foreign key turned off too
GuptaManan100 Nov 9, 2023
9d4263e
comments: add comments explaining the code
GuptaManan100 Nov 9, 2023
e2a3bc1
test: fix test expectations
GuptaManan100 Nov 9, 2023
990d9b1
test: augment the fuzzer to run different foreign key checks mode
GuptaManan100 Nov 9, 2023
3456849
fuzzer: fix fuzzer
GuptaManan100 Nov 9, 2023
8ba85d3
Merge remote-tracking branch 'upstream/main' into fk-checks-vitess-aware
GuptaManan100 Nov 13, 2023
ec3629e
feat: only add fkChecksState field to the plan key if it is non-empty
GuptaManan100 Nov 13, 2023
0d412dd
feat: remove sqlparser.FkChecksState and instead use a *bool to store…
GuptaManan100 Nov 13, 2023
5769be9
feat: use new function to set the foreign key checks value to prevent…
GuptaManan100 Nov 13, 2023
c61108b
Merge remote-tracking branch 'upstream/main' into fk-checks-vitess-aware
GuptaManan100 Nov 16, 2023
24526a5
refactor: extract common code
GuptaManan100 Nov 16, 2023
2d0f11a
test: add failing testcase
GuptaManan100 Nov 16, 2023
495e2c3
feat: fix verify selection query to discount rows that aren't changing
GuptaManan100 Nov 16, 2023
7ad108e
Merge remote-tracking branch 'upstream/main' into fk-checks-vitess-aware
GuptaManan100 Nov 16, 2023
4624a2e
test: update test expectations
GuptaManan100 Nov 16, 2023
06d6ca9
feat: simplify comment parsing
GuptaManan100 Nov 17, 2023
c3230cf
refactor: change prefix used in plan key
GuptaManan100 Nov 20, 2023
c511538
Merge remote-tracking branch 'upstream/main' into fk-checks-vitess-aware
GuptaManan100 Nov 20, 2023
12d409b
refactor: rename Off to OFF and On to ON
GuptaManan100 Nov 20, 2023
f55e95b
feat: remove fk state in plan key
GuptaManan100 Nov 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 73 additions & 44 deletions go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -53,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.
Expand All @@ -72,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,
Expand All @@ -81,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.
Expand Down Expand Up @@ -128,17 +131,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))
}
}

Expand All @@ -155,25 +159,27 @@ 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)
}
}

// generateDeleteDMLQuery generates a DELETE query from the parameters for the fuzzer.
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
}

Expand All @@ -199,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.
Expand Down Expand Up @@ -456,6 +465,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
Expand Down Expand Up @@ -603,49 +627,54 @@ func TestFkFuzzTest(t *testing.T) {
updateShare: 50,
},
}

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.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
}
// 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`")
}

// 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, fkState)

// 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)
})
}
}
}
}
Expand Down Expand Up @@ -701,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))
Expand Down
21 changes: 13 additions & 8 deletions go/test/vschemawrapper/vschema_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down
4 changes: 4 additions & 0 deletions go/vt/schemadiff/semantics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
10 changes: 10 additions & 0 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
30 changes: 20 additions & 10 deletions go/vt/sqlparser/ast_rewriting.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func PrepareAST(
selectLimit int,
setVarComment string,
sysVars map[string]string,
fkChecksState FkChecksState,
views VSchemaViews,
) (*RewriteASTResult, error) {
if parameterize {
Expand All @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
}
Expand All @@ -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)
Expand All @@ -177,13 +181,19 @@ 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 {
newComments := supportOptimizerHint.GetParsedComments().SetMySQLSetVarValue(sysvars.ForeignKeyChecks.Name, er.fkChecksState.String())
supportOptimizerHint.SetComments(newComments)
}
supportOptimizerHint.SetComments(newComments)
}

switch node := cursor.Node().(type) {
Expand Down
Loading
Loading