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

Foreign key cascade planning for DELETE and UPDATE queries #13823

Merged
merged 33 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
23bfeb3
feat: foreign key cascade planner for delete query
harshit-gangal Aug 18, 2023
eefc89c
feat: fix operator planning for fk_cascade and add comments
GuptaManan100 Aug 21, 2023
4a82709
feat: add conversion to logical primitives for fk cascading in delete…
GuptaManan100 Aug 21, 2023
272ec1d
feat: error out for setNull delete action and fix tests
GuptaManan100 Aug 21, 2023
836c04c
feat: fix e2e tests
GuptaManan100 Aug 21, 2023
5f6a3cc
feat: run sizegen
GuptaManan100 Aug 21, 2023
f6e6789
feat: added planning for ON DELETE SET NULL
harshit-gangal Aug 21, 2023
c02be0a
feat: add unit test for cascade SET-NULL delete query
GuptaManan100 Aug 21, 2023
a554bfb
test: augment foreign_key e2e test to verify set null cascading in un…
GuptaManan100 Aug 21, 2023
2c7fdc7
feat:fix the error message for SET DEFAULT type foreign keys in DELETE
GuptaManan100 Aug 21, 2023
06d73ec
test: test for SET DEFAULT
GuptaManan100 Aug 21, 2023
af323bb
feat: add fk on update cascade planner
harshit-gangal Aug 21, 2023
124b458
feat: add fk on update set null planner
harshit-gangal Aug 21, 2023
8d25546
feat: fix e2e tests
GuptaManan100 Aug 22, 2023
22797fd
refactor: perf improvement
GuptaManan100 Aug 22, 2023
91ec17f
refactor: add comments to various places
GuptaManan100 Aug 22, 2023
effb8d4
feat: add more e2e tests
GuptaManan100 Aug 22, 2023
20ac58a
feat: fix set null implementation for update
GuptaManan100 Aug 23, 2023
dac40bf
feat: augment the e2e tests
GuptaManan100 Aug 23, 2023
20eb4cf
test: update test expectations
GuptaManan100 Aug 23, 2023
d1b9756
test: add tests to increase test coverage
GuptaManan100 Aug 23, 2023
4182593
test: add testing for fkNeedsHandlingForUpdates
GuptaManan100 Aug 23, 2023
c70dc24
refactor: improve implementation of fkNeedsHandlingForUpdates to not …
GuptaManan100 Aug 23, 2023
7e589ed
refactor: refactor createOperatorFromUpdate code to extract out commo…
GuptaManan100 Aug 23, 2023
21f7aa5
feat: fix vschema updation with foreign keys and add tests
GuptaManan100 Aug 25, 2023
4220c51
refactor: fix license header
GuptaManan100 Aug 25, 2023
928c1ae
refactor fk cascade planning logic
harshit-gangal Aug 28, 2023
caefaca
test: refactor
harshit-gangal Aug 28, 2023
f15fb52
refactor: FkChild shouldn't be an operator
GuptaManan100 Aug 28, 2023
f92561a
Merge remote-tracking branch 'upstream/main' into fk-cascade-planning
harshit-gangal Aug 28, 2023
c81f64e
fix build
harshit-gangal Aug 28, 2023
6a722e9
refactor of cascade op
harshit-gangal Aug 28, 2023
d6b025b
Merge remote-tracking branch 'upstream/main' into fk-cascade-planning
harshit-gangal Aug 29, 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
1 change: 1 addition & 0 deletions go/mysql/sqlerror/sql_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ var stateToMysqlCode = map[vterrors.State]mysqlCode{
vterrors.ServerNotAvailable: {num: ERServerIsntAvailable, state: SSNetError},
vterrors.CantDoThisInTransaction: {num: ERCantDoThisDuringAnTransaction, state: SSCantDoThisDuringAnTransaction},
vterrors.RequiresPrimaryKey: {num: ERRequiresPrimaryKey, state: SSClientError},
vterrors.RowIsReferenced2: {num: ERRowIsReferenced2, state: SSConstraintViolation},
vterrors.NoSuchSession: {num: ERUnknownComError, state: SSNetError},
vterrors.OperandColumns: {num: EROperandColumns, state: SSWrongNumberOfColumns},
vterrors.WrongValueCountOnRow: {num: ERWrongValueCountOnRow, state: SSWrongValueCountOnRow},
Expand Down
56 changes: 47 additions & 9 deletions go/test/endtoend/vtgate/foreignkey/fk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,26 @@ func TestDeleteWithFK(t *testing.T) {

// table's child foreign key has cross shard fk, so query will fail at vtgate.
_, err = utils.ExecAllowError(t, conn, `delete from t1 where id = 42`)
assert.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)")
assert.ErrorContains(t, err, "VT12002: unsupported: cross-shard foreign keys (errno 1235) (sqlstate 42000)")

// child foreign key is cascade, so query will fail at vtgate.
_, err = utils.ExecAllowError(t, conn, `delete from multicol_tbl1 where cola = 100`)
assert.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)")
// child foreign key is cascade, so this should work as expected.
qr = utils.Exec(t, conn, `delete from multicol_tbl1 where cola = 100`)
assert.EqualValues(t, 1, qr.RowsAffected)
// we also verify that the rows in the child table were deleted.
qr = utils.Exec(t, conn, `select * from multicol_tbl2 where cola = 100`)
assert.Zero(t, qr.Rows)

// Unsharded keyspace tests
utils.Exec(t, conn, `use uks`)
// insert some data.
utils.Exec(t, conn, `insert into u_t1(id, col1) values (100, 123), (10, 12), (1, 13), (1000, 1234)`)
utils.Exec(t, conn, `insert into u_t2(id, col2) values (342, 123), (19, 1234)`)

// Delete from u_t1 which has a foreign key constraint to t2 with SET NULL type.
qr = utils.Exec(t, conn, `delete from u_t1 where id = 100`)
assert.EqualValues(t, 1, qr.RowsAffected)
// Verify the result in u_t2 as well
utils.AssertMatches(t, conn, `select * from u_t2`, `[[INT64(342) NULL] [INT64(19) INT64(1234)]]`)
harshit-gangal marked this conversation as resolved.
Show resolved Hide resolved
}

// TestUpdations tests that update work as expected when foreign key management is enabled in Vitess.
Expand All @@ -91,7 +106,7 @@ func TestUpdateWithFK(t *testing.T) {
// insert some data.
utils.Exec(t, conn, `insert into t1(id, col) values (100, 123),(10, 12),(1, 13),(1000, 1234)`)
utils.Exec(t, conn, `insert into t2(id, col, mycol) values (100, 125, 'foo'), (1, 132, 'bar')`)
utils.Exec(t, conn, `insert into t4(id, col, t2_mycol) values (1, 321, 'bar')`)
utils.Exec(t, conn, `insert into t4(id, col, t2_col, t2_mycol) values (1, 321, 132, 'bar')`)
utils.Exec(t, conn, `insert into t5(pk, sk, col1) values (1, 1, 1),(2, 1, 1),(3, 1, 10),(4, 1, 20),(5, 1, 30),(6, 1, 40)`)
utils.Exec(t, conn, `insert into t6(pk, sk, col1) values (10, 1, 1), (20, 1, 20)`)

Expand All @@ -106,15 +121,38 @@ func TestUpdateWithFK(t *testing.T) {
qr := utils.Exec(t, conn, `update t4 set col = 20 where id = 1`)
assert.EqualValues(t, 1, qr.RowsAffected)

// child table have cascade which is cross shard. Query will fail at vtgate.
_, err = utils.ExecAllowError(t, conn, `update t2 set col = 125 where id = 100`)
assert.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)")

// updating column which does not have foreign key constraint, so query will succeed.
_ = utils.Exec(t, conn, `update t2 set mycol = 'baz' where id = 100`)
assert.EqualValues(t, 1, qr.RowsAffected)

// child table have restrict in shard scoped and value exists in parent table.
_ = utils.Exec(t, conn, `update t6 set col1 = 40 where pk = 20`)
assert.EqualValues(t, 1, qr.RowsAffected)

// Unsharded keyspace tests
utils.Exec(t, conn, `use uks`)
// insert some data.
utils.Exec(t, conn, `insert into u_t1(id, col1) values (100, 123), (10, 12), (1, 13), (1000, 1234)`)
utils.Exec(t, conn, `insert into u_t2(id, col2) values (342, 123), (19, 1234)`)
utils.Exec(t, conn, `insert into u_t3(id, col3) values (32, 123), (1, 12)`)

t.Run("Cascade update with a new value", func(t *testing.T) {
t.Skip("This doesn't work right now. We are able to only cascade updates for which the data already exists in the parent table")
_ = utils.Exec(t, conn, `update u_t1 set col1 = 2 where id = 100`)
})

// Update u_t1 which has a foreign key constraint to u_t2 with SET NULL type, and to u_t3 with CASCADE type.
qr = utils.Exec(t, conn, `update u_t1 set col1 = 13 where id = 100`)
assert.EqualValues(t, 1, qr.RowsAffected)
// Verify the result in u_t2 and u_t3 as well.
utils.AssertMatches(t, conn, `select * from u_t2 order by id`, `[[INT64(19) INT64(1234)] [INT64(342) NULL]]`)
utils.AssertMatches(t, conn, `select * from u_t3 order by id`, `[[INT64(1) INT64(12)] [INT64(32) INT64(13)]]`)

// Update u_t1 which has a foreign key constraint to u_t2 with SET NULL type, and to u_t3 with CASCADE type.
// This update however doesn't change the table.
qr = utils.Exec(t, conn, `update u_t1 set col1 = 1234 where id = 1000`)
assert.EqualValues(t, 0, qr.RowsAffected)
// Verify the result in u_t2 and u_t3 as well.
utils.AssertMatches(t, conn, `select * from u_t2 order by id`, `[[INT64(19) INT64(1234)] [INT64(342) NULL]]`)
utils.AssertMatches(t, conn, `select * from u_t3 order by id`, `[[INT64(1) INT64(12)] [INT64(32) INT64(13)]]`)
}
27 changes: 26 additions & 1 deletion go/test/endtoend/vtgate/foreignkey/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,19 @@ var (
clusterInstance *cluster.LocalProcessCluster
vtParams mysql.ConnParams
shardedKs = "ks"
unshardedKs = "uks"
Cell = "test"

//go:embed sharded_schema.sql
shardedSchemaSQL string

//go:embed unsharded_schema.sql
unshardedSchemaSQL string

//go:embed sharded_vschema.json
shardedVSchema string

//go:embed unsharded_vschema.json
unshardedVSchema string
)

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -70,6 +76,21 @@ func TestMain(m *testing.M) {
return 1
}

uKs := &cluster.Keyspace{
Name: unshardedKs,
SchemaSQL: unshardedSchemaSQL,
VSchema: unshardedVSchema,
}
err = clusterInstance.StartUnshardedKeyspace(*uKs, 0, false)
if err != nil {
return 1
}

err = clusterInstance.VtctlclientProcess.ExecuteCommand("RebuildVSchemaGraph")
if err != nil {
return 1
}

// Start vtgate
err = clusterInstance.StartVtgate()
if err != nil {
Expand Down Expand Up @@ -99,6 +120,10 @@ func start(t *testing.T) (*mysql.Conn, func()) {
for _, table := range tables {
_ = utils.Exec(t, conn, "delete from "+table)
}
_ = utils.Exec(t, conn, "use `uks`")
for _, table := range []string{"u_t1", "u_t2"} {
_ = utils.Exec(t, conn, "delete from "+table)
}
_ = utils.Exec(t, conn, "use `ks`")
}

Expand Down
23 changes: 23 additions & 0 deletions go/test/endtoend/vtgate/foreignkey/unsharded_schema.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
create table u_t1
(
id bigint,
col1 bigint,
index(col1),
primary key (id)
) Engine = InnoDB;

create table u_t2
(
id bigint,
col2 bigint,
primary key (id),
foreign key (col2) references u_t1 (col1) on delete set null on update set null
) Engine = InnoDB;

create table u_t3
(
id bigint,
col3 bigint,
primary key (id),
foreign key (col3) references u_t1 (col1) on delete cascade on update cascade
) Engine = InnoDB;
8 changes: 8 additions & 0 deletions go/test/endtoend/vtgate/foreignkey/unsharded_vschema.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"sharded": false,
"foreignKeyMode": "FK_MANAGED",
"tables": {
"u_a": {},
"u_b": {}
}
}
2 changes: 2 additions & 0 deletions go/vt/vterrors/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ var (
VT09013 = errorWithoutState("VT09013", vtrpcpb.Code_FAILED_PRECONDITION, "semi-sync plugins are not loaded", "Durability policy wants Vitess to use semi-sync, but the MySQL instances don't have the semi-sync plugin loaded.")
VT09014 = errorWithoutState("VT09014", vtrpcpb.Code_FAILED_PRECONDITION, "vindex cannot be modified", "The vindex cannot be used as table in DML statement")
VT09015 = errorWithoutState("VT09015", vtrpcpb.Code_FAILED_PRECONDITION, "schema tracking required", "This query cannot be planned without more information on the SQL schema. Please turn on schema tracking or add authoritative columns information to your VSchema.")
VT09016 = errorWithState("VT09016", vtrpcpb.Code_FAILED_PRECONDITION, RowIsReferenced2, "Cannot delete or update a parent row: a foreign key constraint fails", "SET DEFAULT is not supported by InnoDB")

VT10001 = errorWithoutState("VT10001", vtrpcpb.Code_ABORTED, "foreign key constraints are not allowed", "Foreign key constraints are not allowed, see https://vitess.io/blog/2021-06-15-online-ddl-why-no-fk/.")

Expand Down Expand Up @@ -143,6 +144,7 @@ var (
VT09013,
VT09014,
VT09015,
VT09016,
VT10001,
VT12001,
VT12002,
Expand Down
1 change: 1 addition & 0 deletions go/vt/vterrors/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const (
CantDoThisInTransaction
RequiresPrimaryKey
OperandColumns
RowIsReferenced2
harshit-gangal marked this conversation as resolved.
Show resolved Hide resolved
UnknownStmtHandler

// not found
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vtgate/engine/cached_size.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 20 additions & 7 deletions go/vt/vtgate/engine/fk_cascade.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type FkCascade struct {
// Selection is the Primitive that is used to find the rows that are going to be modified in the child tables.
Selection Primitive
// Children is a list of child foreign key Primitives that are executed using rows from the Selection Primitive.
Children []FkChild
Children []*FkChild
// Parent is the Primitive that is executed after the children are modified.
Parent Primitive

Expand Down Expand Up @@ -168,17 +168,30 @@ func (fkc *FkCascade) TryStreamExecute(ctx context.Context, vcursor VCursor, bin

// Inputs implements the Primitive interface.
func (fkc *FkCascade) Inputs() []Primitive {
inputs := []Primitive{fkc.Selection}
for _, child := range fkc.Children {
inputs = append(inputs, child.Exec)
}
inputs = append(inputs, fkc.Parent)
return inputs
return nil
}

func (fkc *FkCascade) description() PrimitiveDescription {
var childrenDesc []PrimitiveDescription
for _, child := range fkc.Children {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need to visit children here. this is done outside, from the caller of this method

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't put the fkc.Children in the Inputs() output so we don't actually visit them from the caller of this method.
The reason we do it this way is because we want to print the BvName and Cols fields too which otherwise aren't printed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed it offline, and we'll change this. cc - @harshit-gangal

childrenDesc = append(childrenDesc, PrimitiveDescription{
OperatorType: "FkCascadeChild",
Inputs: []PrimitiveDescription{
PrimitiveToPlanDescription(child.Exec),
},
Other: map[string]any{
"BvName": child.BVName,
"Cols": child.Cols,
},
})
}
return PrimitiveDescription{
OperatorType: fkc.RouteType(),
Other: map[string]any{
"Selection": PrimitiveToPlanDescription(fkc.Selection),
"Parent": PrimitiveToPlanDescription(fkc.Parent),
"Children": childrenDesc,
},
}
}

Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/engine/fk_cascade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestDeleteCascade(t *testing.T) {
}
fkc := &FkCascade{
Selection: inputP,
Children: []FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}},
Children: []*FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}},
Parent: parentP,
}

Expand Down Expand Up @@ -119,7 +119,7 @@ func TestUpdateCascade(t *testing.T) {
}
fkc := &FkCascade{
Selection: inputP,
Children: []FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}},
Children: []*FkChild{{BVName: "__vals", Cols: []int{0, 1}, Exec: childP}},
Parent: parentP,
}

Expand Down
17 changes: 5 additions & 12 deletions go/vt/vtgate/planbuilder/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,46 +46,39 @@ func gen4DeleteStmtPlanner(
}
}

ksName := ""
if ks, _ := vschema.DefaultKeyspace(); ks != nil {
ksName = ks.Name
}
semTable, err := semantics.Analyze(deleteStmt, ksName, vschema)
ctx, err := plancontext.CreatePlanningContext(deleteStmt, reservedVars, vschema, version)
if err != nil {
return nil, err
}

// record any warning as planner warning.
vschema.PlannerWarning(semTable.Warning)
err = rewriteRoutedTables(deleteStmt, vschema)
if err != nil {
return nil, err
}

if ks, tables := semTable.SingleUnshardedKeyspace(); ks != nil {
if ks, tables := ctx.SemTable.SingleUnshardedKeyspace(); ks != nil {
if fkManagementNotRequired(vschema, tables) {
plan := deleteUnshardedShortcut(deleteStmt, ks, tables)
plan = pushCommentDirectivesOnPlan(plan, deleteStmt)
return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil
}
}

if err := checkIfDeleteSupported(deleteStmt, semTable); err != nil {
if err := checkIfDeleteSupported(deleteStmt, ctx.SemTable); err != nil {
return nil, err
}

err = queryRewrite(semTable, reservedVars, deleteStmt)
err = queryRewrite(ctx.SemTable, reservedVars, deleteStmt)
if err != nil {
return nil, err
}

ctx := plancontext.NewPlanningContext(reservedVars, semTable, vschema, version)
op, err := operators.PlanQuery(ctx, deleteStmt)
if err != nil {
return nil, err
}

plan, err := transformToLogicalPlan(ctx, op, true)
plan, err := transformToLogicalPlan(ctx, op)
if err != nil {
return nil, err
}
Expand Down
Loading
Loading