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 Keys: DELETE planning #13746

Merged
merged 4 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions go/test/endtoend/vtgate/foreignkey/fk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,51 @@ func TestInsertions(t *testing.T) {

// Verify that inserting data into a table that has shard scoped foreign keys works.
utils.Exec(t, conn, `insert into t2(id, col) values (100, 125), (1, 132)`)

// Verify that insertion fails if the data doesn't follow the fk constraint.
_, err := utils.ExecAllowError(t, conn, `insert into t2(id, col) values (1310, 125)`)
require.ErrorContains(t, err, "Cannot add or update a child row: a foreign key constraint fails")

// Verify that insertion fails if the table has cross-shard foreign keys (even if the data follows the constraints).
_, err = utils.ExecAllowError(t, conn, `insert into t3(id, col) values (100, 100)`)
require.ErrorContains(t, err, "VT12002: unsupported: cross-shard foreign keys")

// insert some data in a table with multicol vindex.
utils.Exec(t, conn, `insert into multicol_tbl1(cola, colb, colc, msg) values (100, 'a', 'b', 'msg'), (101, 'c', 'd', 'msg2')`)

// Verify that inserting data into a table that has shard scoped multi-column foreign keys works.
utils.Exec(t, conn, `insert into multicol_tbl2(cola, colb, colc, msg) values (100, 'a', 'b', 'msg3')`)

// Verify that insertion fails if the data doesn't follow the fk constraint.
_, err = utils.ExecAllowError(t, conn, `insert into multicol_tbl2(cola, colb, colc, msg) values (103, 'c', 'd', 'msg2')`)
require.ErrorContains(t, err, "Cannot add or update a child row: a foreign key constraint fails")
}

// TestDeletions tests that deletions work as expected when foreign key management is enabled in Vitess.
func TestDeletions(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nicely documented

conn, closer := start(t)
defer closer()

// 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) values (100, 125), (1, 132)`)
utils.Exec(t, conn, `insert into t4(id, col) values (1, 321)`)
utils.Exec(t, conn, `insert into multicol_tbl1(cola, colb, colc, msg) values (100, 'a', 'b', 'msg'), (101, 'c', 'd', 'msg2')`)
utils.Exec(t, conn, `insert into multicol_tbl2(cola, colb, colc, msg) values (100, 'a', 'b', 'msg3')`)

// child foreign key is shard scoped. Query will fail at mysql due to On Delete Restrict.
_, err := utils.ExecAllowError(t, conn, `delete from t2 where col = 132`)
require.ErrorContains(t, err, "Cannot delete or update a parent row: a foreign key constraint fails")

// child row does not exist so query will succeed.
qr := utils.Exec(t, conn, `delete from t2 where col = 125`)
require.EqualValues(t, 1, qr.RowsAffected)

// 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`)
require.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (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`)
require.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)")
}
8 changes: 7 additions & 1 deletion go/test/endtoend/vtgate/foreignkey/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,16 @@ func start(t *testing.T) (*mysql.Conn, func()) {
require.NoError(t, err)

deleteAll := func() {
tables := []string{"t3", "t2", "t1", "multicol_tbl2", "multicol_tbl1"}
_ = utils.Exec(t, conn, "use `ks/-80`")
tables := []string{"t4", "t3", "t2", "t1", "multicol_tbl2", "multicol_tbl1"}
for _, table := range tables {
_ = utils.Exec(t, conn, "delete from "+table)
}
_ = utils.Exec(t, conn, "use `ks/80-`")
for _, table := range tables {
_ = utils.Exec(t, conn, "delete from "+table)
}
_ = utils.Exec(t, conn, "use `ks`")
}

deleteAll()
Expand Down
14 changes: 11 additions & 3 deletions go/test/endtoend/vtgate/foreignkey/sharded_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ create table t2
id bigint,
col bigint,
primary key (id),
foreign key (id) references t1 (id)
foreign key (id) references t1 (id) on delete restrict
) Engine = InnoDB;

create table t3
(
id bigint,
col bigint,
primary key (id),
foreign key (col) references t1 (id)
foreign key (col) references t1 (id) on delete restrict
) Engine = InnoDB;

create table multicol_tbl1
Expand All @@ -37,5 +37,13 @@ create table multicol_tbl2
colc varchar(50),
msg varchar(50),
primary key (cola, colb, colc),
foreign key (cola, colb, colc) references multicol_tbl1 (cola, colb, colc)
foreign key (cola, colb, colc) references multicol_tbl1 (cola, colb, colc) on delete cascade
) Engine = InnoDB;

create table t4
(
id bigint,
col bigint,
primary key (id),
foreign key (id) references t2 (id) on delete restrict
) Engine = InnoDB;
8 changes: 8 additions & 0 deletions go/test/endtoend/vtgate/foreignkey/sharded_vschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@
}
]
},
"t4": {
"column_vindexes": [
{
"column": "id",
"name": "xxhash"
}
]
},
"multicol_tbl1": {
"column_vindexes": [
{
Expand Down
2 changes: 2 additions & 0 deletions go/vt/vterrors/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ var (

VT12001 = errorWithoutState("VT12001", vtrpcpb.Code_UNIMPLEMENTED, "unsupported: %s", "This statement is unsupported by Vitess. Please rewrite your query to use supported syntax.")
VT12002 = errorWithoutState("VT12002", vtrpcpb.Code_UNIMPLEMENTED, "unsupported: cross-shard foreign keys", "Vitess does not support cross shard foreign keys.")
VT12003 = errorWithoutState("VT12002", vtrpcpb.Code_UNIMPLEMENTED, "unsupported: foreign keys management at vitess", "Vitess does not support managing foreign keys tables.")

// VT13001 General Error
VT13001 = errorWithoutState("VT13001", vtrpcpb.Code_INTERNAL, "[BUG] %s", "This error should not happen and is a bug. Please file an issue on GitHub: https://github.com/vitessio/vitess/issues/new/choose.")
Expand Down Expand Up @@ -145,6 +146,7 @@ var (
VT10001,
VT12001,
VT12002,
VT12003,
VT13001,
VT13002,
VT14001,
Expand Down
27 changes: 24 additions & 3 deletions go/vt/vtgate/planbuilder/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package planbuilder

import (
querypb "vitess.io/vitess/go/vt/proto/query"
vschemapb "vitess.io/vitess/go/vt/proto/vschema"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vterrors"
"vitess.io/vitess/go/vt/vtgate/engine"
Expand Down Expand Up @@ -62,9 +63,11 @@ func gen4DeleteStmtPlanner(
}

if ks, tables := semTable.SingleUnshardedKeyspace(); ks != nil {
plan := deleteUnshardedShortcut(deleteStmt, ks, tables)
plan = pushCommentDirectivesOnPlan(plan, deleteStmt)
return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), 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 {
Expand Down Expand Up @@ -98,6 +101,24 @@ func gen4DeleteStmtPlanner(
return newPlanResult(plan.Primitive(), operators.TablesUsed(op)...), nil
}

func fkManagementNotRequired(vschema plancontext.VSchema, vTables []*vindexes.Table) bool {
// Find the foreign key mode and check for any managed child foreign keys.
for _, vTable := range vTables {
ksMode, err := vschema.ForeignKeyMode(vTable.Keyspace.Name)
if err != nil {
return false
}
if ksMode != vschemapb.Keyspace_FK_MANAGED {
continue
}
childFks := vTable.ChildFKsNeedsHandling()
if len(childFks) > 0 {
return false
}
}
return true
}

func rewriteSingleTbl(del *sqlparser.Delete) (*sqlparser.Delete, error) {
atExpr, ok := del.TableExprs[0].(*sqlparser.AliasedTableExpr)
if !ok {
Expand Down
11 changes: 11 additions & 0 deletions go/vt/vtgate/planbuilder/operators/ast2op.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,17 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp
Routing: routing,
}

ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name)
if err != nil {
return nil, err
}
if ksMode == vschemapb.Keyspace_FK_MANAGED {
childFks := vindexTable.ChildFKsNeedsHandling()
if len(childFks) > 0 {
return nil, vterrors.VT12003()
}
}

if !vindexTable.Keyspace.Sharded {
return route, nil
}
Expand Down
16 changes: 12 additions & 4 deletions go/vt/vtgate/planbuilder/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,25 +435,33 @@ func loadSchema(t testing.TB, filename string, setCollation bool) *vindexes.VSch
}
if vschema.Keyspaces["user_fk_allow"] != nil {
// FK from multicol_tbl2 referencing multicol_tbl1 that is shard scoped.
err = vschema.AddForeignKey("user_fk_allow", "multicol_tbl2", createFkDefinition([]string{"colb", "cola", "x", "colc", "y"}, "multicol_tbl1", []string{"colb", "cola", "y", "colc", "x"}))
err = vschema.AddForeignKey("user_fk_allow", "multicol_tbl2", createFkDefinition([]string{"colb", "cola", "x", "colc", "y"}, "multicol_tbl1", []string{"colb", "cola", "y", "colc", "x"}, sqlparser.Cascade, sqlparser.Cascade))
require.NoError(t, err)
// FK from tbl2 referencing tbl1 that is shard scoped.
err = vschema.AddForeignKey("user_fk_allow", "tbl2", createFkDefinition([]string{"col2"}, "tbl1", []string{"col1"}))
err = vschema.AddForeignKey("user_fk_allow", "tbl2", createFkDefinition([]string{"col2"}, "tbl1", []string{"col1"}, sqlparser.Restrict, sqlparser.Restrict))
require.NoError(t, err)
// FK from tbl3 referencing tbl1 that is not shard scoped.
err = vschema.AddForeignKey("user_fk_allow", "tbl3", createFkDefinition([]string{"coly"}, "tbl1", []string{"col1"}))
err = vschema.AddForeignKey("user_fk_allow", "tbl3", createFkDefinition([]string{"coly"}, "tbl1", []string{"col1"}, sqlparser.DefaultAction, sqlparser.DefaultAction))
require.NoError(t, err)
// FK from tbl4 referencing tbl5 that is shard scoped.
err = vschema.AddForeignKey("user_fk_allow", "tbl4", createFkDefinition([]string{"col4"}, "tbl5", []string{"col5"}, sqlparser.SetNull, sqlparser.SetNull))
require.NoError(t, err)
// FK from tbl6 referencing tbl7 that is shard scoped.
err = vschema.AddForeignKey("user_fk_allow", "tbl6", createFkDefinition([]string{"col6"}, "tbl7", []string{"col7"}, sqlparser.NoAction, sqlparser.NoAction))
require.NoError(t, err)
}
return vschema
}

// createFkDefinition is a helper function to create a Foreign key definition struct from the columns used in it provided as list of strings.
func createFkDefinition(childCols []string, parentTableName string, parentCols []string) *sqlparser.ForeignKeyDefinition {
func createFkDefinition(childCols []string, parentTableName string, parentCols []string, onUpdate, onDelete sqlparser.ReferenceAction) *sqlparser.ForeignKeyDefinition {
return &sqlparser.ForeignKeyDefinition{
Source: sqlparser.MakeColumns(childCols...),
ReferenceDefinition: &sqlparser.ReferenceDefinition{
ReferencedTable: sqlparser.NewTableName(parentTableName),
ReferencedColumns: sqlparser.MakeColumns(parentCols...),
OnUpdate: onUpdate,
OnDelete: onDelete,
},
}
}
Expand Down
37 changes: 37 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,42 @@
"user_fk_allow.multicol_tbl2"
]
}
},
{
"comment": "Delete in a table with cross-shard foreign keys disallowed",
"query": "delete from tbl1",
"plan": "VT12002: unsupported: foreign keys management at vitess"
},
{
"comment": "Delete in a table with shard-scoped foreign keys is allowed",
"query": "delete from tbl7",
"plan": {
"QueryType": "DELETE",
"Original": "delete from tbl7",
"Instructions": {
"OperatorType": "Delete",
"Variant": "Scatter",
"Keyspace": {
"Name": "user_fk_allow",
"Sharded": true
},
"TargetTabletType": "PRIMARY",
"Query": "delete from tbl7",
"Table": "tbl7"
},
"TablesUsed": [
"user_fk_allow.tbl7"
]
}
},
{
"comment": "Delete in a table with shard-scoped multiple column foreign key with cascade not allowed",
"query": "delete from multicol_tbl1 where cola = 1 and colb = 2 and colc = 3",
"plan": "VT12002: unsupported: foreign keys management at vitess"
},
{
"comment": "Delete in a table with shard-scoped foreign keys with cascade disallowed",
"query": "delete from tbl5",
"plan": "VT12002: unsupported: foreign keys management at vitess"
}
]
35 changes: 33 additions & 2 deletions go/vt/vtgate/planbuilder/testdata/vschemas/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -608,8 +608,7 @@
"foreignKeyMode": "FK_MANAGED",
"vindexes": {
"hash_vin": {
"type": "hash_test",
"owner": "user"
"type": "hash_test"
},
"multicolIdx": {
"type": "multiCol_test"
Expand Down Expand Up @@ -663,6 +662,38 @@
"name": "hash_vin"
}
]
},
"tbl4": {
"column_vindexes": [
{
"column": "col4",
"name": "hash_vin"
}
]
},
"tbl5": {
"column_vindexes": [
{
"column": "col5",
"name": "hash_vin"
}
]
},
"tbl6": {
"column_vindexes": [
{
"column": "col6",
"name": "hash_vin"
}
]
},
"tbl7": {
"column_vindexes": [
{
"column": "col7",
"name": "hash_vin"
}
]
}
}
}
Expand Down
Loading
Loading