From e84a2dbb6806ae1371008c12ab6de69dd29d815f Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 17 Aug 2023 11:58:49 +0530 Subject: [PATCH] Foreign Keys: `UPDATE` planning (#13762) Signed-off-by: Harshit Gangal Signed-off-by: Andres Taylor Co-authored-by: Andres Taylor --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 60 ++++++-- .../vtgate/foreignkey/sharded_schema.sql | 35 ++++- .../vtgate/foreignkey/sharded_vschema.json | 16 +++ go/vt/vtgate/planbuilder/delete.go | 2 +- go/vt/vtgate/planbuilder/operators/ast2op.go | 37 ++++- go/vt/vtgate/planbuilder/plan_test.go | 66 ++++++--- .../testdata/foreignkey_cases.json | 133 +++++++++++++++--- .../planbuilder/testdata/vschemas/schema.json | 21 ++- go/vt/vtgate/planbuilder/update.go | 41 +++++- go/vt/vtgate/vindexes/foreign_keys.go | 18 +-- go/vt/vtgate/vindexes/foreign_keys_test.go | 5 +- 11 files changed, 360 insertions(+), 74 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index 2478b525508..531aed2642a 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -19,13 +19,13 @@ package foreignkey import ( "testing" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" "vitess.io/vitess/go/test/endtoend/utils" ) -// TestInsertions tests that insertions work as expected when foreign key management is enabled in Vitess. -func TestInsertions(t *testing.T) { +// TestInsertWithFK tests that insertions work as expected when foreign key management is enabled in Vitess. +func TestInsertWithFK(t *testing.T) { conn, closer := start(t) defer closer() @@ -37,11 +37,11 @@ func TestInsertions(t *testing.T) { // 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") + assert.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") + assert.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')`) @@ -51,11 +51,11 @@ func TestInsertions(t *testing.T) { // 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") + assert.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) { +// TestDeleteWithFK tests that deletions work as expected when foreign key management is enabled in Vitess. +func TestDeleteWithFK(t *testing.T) { conn, closer := start(t) defer closer() @@ -68,17 +68,53 @@ func TestDeletions(t *testing.T) { // 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") + assert.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) + assert.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)") + assert.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)") + assert.ErrorContains(t, err, "VT12002: unsupported: foreign keys management at vitess (errno 1235) (sqlstate 42000)") +} + +// TestUpdations tests that update work as expected when foreign key management is enabled in Vitess. +func TestUpdateWithFK(t *testing.T) { + 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, 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 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)`) + + // parent foreign key is shard scoped and value is not updated. Query will succeed. + _ = utils.Exec(t, conn, `update t4 set t2_mycol = 'bar' where id = 1`) + + // parent foreign key is shard scoped and value does not exists in parent table. Query will fail at mysql due to On Update Restrict. + _, err := utils.ExecAllowError(t, conn, `update t4 set t2_mycol = 'foo' where id = 1`) + assert.ErrorContains(t, err, "Cannot add or update a child row: a foreign key constraint fails") + + // updating column which does not have foreign key constraint, so query will succeed. + 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) } diff --git a/go/test/endtoend/vtgate/foreignkey/sharded_schema.sql b/go/test/endtoend/vtgate/foreignkey/sharded_schema.sql index 37b9e5c15c6..8475104fecc 100644 --- a/go/test/endtoend/vtgate/foreignkey/sharded_schema.sql +++ b/go/test/endtoend/vtgate/foreignkey/sharded_schema.sql @@ -9,14 +9,17 @@ create table t2 ( id bigint, col bigint, + mycol varchar(50), primary key (id), + index(id, mycol), + index(id, col), foreign key (id) references t1 (id) on delete restrict ) Engine = InnoDB; create table t3 ( - id bigint, - col bigint, + id bigint, + col bigint, primary key (id), foreign key (col) references t1 (id) on delete restrict ) Engine = InnoDB; @@ -42,8 +45,30 @@ create table multicol_tbl2 create table t4 ( - id bigint, - col bigint, + id bigint, + col bigint, + t2_mycol varchar(50), + t2_col bigint, primary key (id), - foreign key (id) references t2 (id) on delete restrict + foreign key (id) references t2 (id) on delete restrict, + foreign key (id, t2_mycol) references t2 (id, mycol) on update restrict, + foreign key (id, t2_col) references t2 (id, col) on update cascade +) Engine = InnoDB; + +create table t5 +( + pk bigint, + sk bigint, + col1 varchar(50), + primary key (pk), + index(sk, col1) +) Engine = InnoDB; + +create table t6 +( + pk bigint, + sk bigint, + col1 varchar(50), + primary key (pk), + foreign key (sk, col1) references t5 (sk, col1) on delete restrict on update restrict ) Engine = InnoDB; diff --git a/go/test/endtoend/vtgate/foreignkey/sharded_vschema.json b/go/test/endtoend/vtgate/foreignkey/sharded_vschema.json index 04127774fd1..c80662b6852 100644 --- a/go/test/endtoend/vtgate/foreignkey/sharded_vschema.json +++ b/go/test/endtoend/vtgate/foreignkey/sharded_vschema.json @@ -47,6 +47,22 @@ } ] }, + "t5": { + "column_vindexes": [ + { + "column": "sk", + "name": "xxhash" + } + ] + }, + "t6": { + "column_vindexes": [ + { + "column": "sk", + "name": "xxhash" + } + ] + }, "multicol_tbl1": { "column_vindexes": [ { diff --git a/go/vt/vtgate/planbuilder/delete.go b/go/vt/vtgate/planbuilder/delete.go index cef2108824e..4fae6ae97fa 100644 --- a/go/vt/vtgate/planbuilder/delete.go +++ b/go/vt/vtgate/planbuilder/delete.go @@ -111,7 +111,7 @@ func fkManagementNotRequired(vschema plancontext.VSchema, vTables []*vindexes.Ta if ksMode != vschemapb.Keyspace_FK_MANAGED { continue } - childFks := vTable.ChildFKsNeedsHandling() + childFks := vTable.ChildFKsNeedsHandling(vindexes.DeleteAction) if len(childFks) > 0 { return false } diff --git a/go/vt/vtgate/planbuilder/operators/ast2op.go b/go/vt/vtgate/planbuilder/operators/ast2op.go index 24e0de66a0a..a4463035d67 100644 --- a/go/vt/vtgate/planbuilder/operators/ast2op.go +++ b/go/vt/vtgate/planbuilder/operators/ast2op.go @@ -160,6 +160,21 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars Routing: routing, } + ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) + if err != nil { + return nil, err + } + if ksMode == vschemapb.Keyspace_FK_MANAGED { + parentFKs := vindexTable.ParentFKsNeedsHandling() + childFks := vindexTable.ChildFKsNeedsHandling(vindexes.UpdateAction) + if (len(childFks) > 0 || len(parentFKs) > 0) && + ColumnModified(updStmt.Exprs, func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { + return parentFKs, childFks + }) { + return nil, vterrors.VT12003() + } + } + subq, err := createSubqueryFromStatement(ctx, updStmt) if err != nil { return nil, err @@ -171,6 +186,24 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return subq, nil } +// ColumnModified checks if any column in the parent table is being updated which has a child foreign key. +func ColumnModified(exprs sqlparser.UpdateExprs, getFks func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo)) bool { + for _, updateExpr := range exprs { + parentFKs, childFks := getFks(updateExpr) + for _, childFk := range childFks { + if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { + return true + } + } + for _, parentFk := range parentFKs { + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + return true + } + } + } + return false +} + func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete) (ops.Operator, error) { tableInfo, qt, err := createQueryTableForDML(ctx, deleteStmt.TableExprs[0], deleteStmt.Where) if err != nil { @@ -197,7 +230,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp return nil, err } if ksMode == vschemapb.Keyspace_FK_MANAGED { - childFks := vindexTable.ChildFKsNeedsHandling() + childFks := vindexTable.ChildFKsNeedsHandling(vindexes.DeleteAction) if len(childFks) > 0 { return nil, vterrors.VT12003() } @@ -279,7 +312,7 @@ func createOperatorFromInsert(ctx *plancontext.PlanningContext, ins *sqlparser.I return nil, err } if ksMode == vschemapb.Keyspace_FK_MANAGED { - parentFKs := vindexTable.CrossShardParentFKs() + parentFKs := vindexTable.ParentFKsNeedsHandling() if len(parentFKs) > 0 { return nil, vterrors.VT12002() } diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 0265a65619e..c548f1c0ca9 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -99,19 +99,56 @@ func TestPlan(t *testing.T) { // TestForeignKeyPlanning tests the planning of foreign keys in a managed mode by Vitess. func TestForeignKeyPlanning(t *testing.T) { + vschema := loadSchema(t, "vschemas/schema.json", true) + setFks(t, vschema) vschemaWrapper := &vschemawrapper.VSchemaWrapper{ - V: loadSchema(t, "vschemas/schema.json", true), - // Set the keyspace with foreign keys enabled as the default. - Keyspace: &vindexes.Keyspace{ - Name: "user_fk_allow", - Sharded: true, - }, + V: vschema, } + testOutputTempDir := makeTestOutput(t) testFile(t, "foreignkey_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. + _ = vschema.AddForeignKey("sharded_fk_allow", "multicol_tbl2", createFkDefinition([]string{"colb", "cola", "x", "colc", "y"}, "multicol_tbl1", []string{"colb", "cola", "y", "colc", "x"}, sqlparser.Cascade, sqlparser.Cascade)) + + // FK from tbl2 referencing tbl1 that is shard scoped. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl2", createFkDefinition([]string{"col2"}, "tbl1", []string{"col1"}, sqlparser.Restrict, sqlparser.Restrict)) + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl2", createFkDefinition([]string{"col2", "col"}, "tbl1", []string{"col1", "col"}, sqlparser.Restrict, sqlparser.Restrict)) + // FK from tbl3 referencing tbl1 that is not shard scoped. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl3", createFkDefinition([]string{"coly"}, "tbl1", []string{"t1col1"}, sqlparser.DefaultAction, sqlparser.DefaultAction)) + // FK from tbl10 referencing tbl2 that is shard scoped. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl10", createFkDefinition([]string{"sk", "col"}, "tbl2", []string{"col2", "col"}, sqlparser.Restrict, sqlparser.Restrict)) + // FK from tbl10 referencing tbl3 that is not shard scoped. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl10", createFkDefinition([]string{"col"}, "tbl3", []string{"col"}, sqlparser.Restrict, sqlparser.Restrict)) + + // FK from tbl4 referencing tbl5 that is shard scoped. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"col4"}, "tbl5", []string{"col5"}, sqlparser.SetNull, sqlparser.SetNull)) + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl4", createFkDefinition([]string{"t4col4"}, "tbl5", []string{"t5col5"}, sqlparser.SetNull, sqlparser.SetNull)) + + // FK from tbl6 referencing tbl7 that is shard scoped. + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"col6"}, "tbl7", []string{"col7"}, sqlparser.NoAction, sqlparser.NoAction)) + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"t6col6"}, "tbl7", []string{"t7col7"}, sqlparser.NoAction, sqlparser.NoAction)) + _ = vschema.AddForeignKey("sharded_fk_allow", "tbl6", createFkDefinition([]string{"t6col62"}, "tbl7", []string{"t7col72"}, sqlparser.NoAction, sqlparser.NoAction)) + } + if vschema.Keyspaces["unsharded_fk_allow"] != nil { + // u_tbl2(col2) -> u_tbl1(col1) Cascade. + // u_tbl3(col2) -> u_tbl2(col2) Cascade Null. + // u_tbl4(col41) -> u_tbl1(col14) Restrict. + // u_tbl4(col4) -> u_tbl3(col3) Restrict. + // u_tbl6(col6) -> u_tbl5(col5) Restrict. + + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl2", createFkDefinition([]string{"col2"}, "u_tbl1", []string{"col1"}, sqlparser.Cascade, sqlparser.Cascade)) + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl3", createFkDefinition([]string{"col3"}, "u_tbl2", []string{"col2"}, sqlparser.SetNull, sqlparser.SetNull)) + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl4", createFkDefinition([]string{"col41"}, "u_tbl1", []string{"col14"}, sqlparser.NoAction, sqlparser.NoAction)) + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl4", createFkDefinition([]string{"col4"}, "u_tbl3", []string{"col3"}, sqlparser.Restrict, sqlparser.Restrict)) + _ = vschema.AddForeignKey("unsharded_fk_allow", "u_tbl6", createFkDefinition([]string{"col6"}, "u_tbl5", []string{"col5"}, sqlparser.DefaultAction, sqlparser.DefaultAction)) + } +} + func TestSystemTables57(t *testing.T) { // first we move everything to use 5.7 logic oldVer := servenv.MySQLServerVersion() @@ -436,23 +473,6 @@ 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"}, 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"}, 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"}, 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 } diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index aefdd6dfb50..f316dd68e92 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -14,7 +14,7 @@ "OperatorType": "Insert", "Variant": "Sharded", "Keyspace": { - "Name": "user_fk_allow", + "Name": "sharded_fk_allow", "Sharded": true }, "TargetTabletType": "PRIMARY", @@ -25,7 +25,7 @@ } }, "TablesUsed": [ - "user_fk_allow.tbl2" + "sharded_fk_allow.tbl2" ] } }, @@ -39,7 +39,7 @@ "OperatorType": "Insert", "Variant": "Sharded", "Keyspace": { - "Name": "user_fk_allow", + "Name": "sharded_fk_allow", "Sharded": true }, "TargetTabletType": "PRIMARY", @@ -50,7 +50,7 @@ } }, "TablesUsed": [ - "user_fk_allow.multicol_tbl2" + "sharded_fk_allow.multicol_tbl2" ] } }, @@ -60,35 +60,136 @@ "plan": "VT12002: unsupported: foreign keys management at vitess" }, { - "comment": "Delete in a table with shard-scoped foreign keys is allowed", + "comment": "Delete in a table with not all column shard-scoped foreign keys - disallowed", "query": "delete from tbl7", + "plan": "VT12002: unsupported: foreign keys management at vitess" + }, + { + "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" + }, + { + "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 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": "VT12002: unsupported: foreign keys management at vitess" + }, + { + "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 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: foreign keys management at vitess" + }, + { + "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": "DELETE", - "Original": "delete from tbl7", + "QueryType": "UPDATE", + "Original": "update tbl1 set not_ref_col = 'foo' where id = 1", "Instructions": { - "OperatorType": "Delete", + "OperatorType": "Update", "Variant": "Scatter", "Keyspace": { - "Name": "user_fk_allow", + "Name": "sharded_fk_allow", "Sharded": true }, "TargetTabletType": "PRIMARY", - "Query": "delete from tbl7", - "Table": "tbl7" + "Query": "update tbl1 set not_ref_col = 'foo' where id = 1", + "Table": "tbl1" }, "TablesUsed": [ - "user_fk_allow.tbl7" + "sharded_fk_allow.tbl1" ] } }, { - "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", + "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: foreign keys management at vitess" }, { - "comment": "Delete in a table with shard-scoped foreign keys with cascade disallowed", - "query": "delete from tbl5", + "comment": "Update in a table with shard-scoped foreign keys with cascade disallowed", + "query": "update tbl5 set t5col5 = 'foo'", + "plan": "VT12002: unsupported: foreign keys management at vitess" + }, + { + "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 tbl2 set col = 'foo'", + "Table": "tbl2" + }, + "TablesUsed": [ + "sharded_fk_allow.tbl2" + ] + } + }, + { + "comment": "update table with column's parent foreign key cross shard - disallowed", + "query": "update tbl10 set col = 'foo'", "plan": "VT12002: unsupported: foreign keys management at vitess" } ] \ No newline at end of file diff --git a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json index 52f1c8da7a8..0b474163ce5 100644 --- a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json +++ b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json @@ -603,7 +603,7 @@ } } }, - "user_fk_allow": { + "sharded_fk_allow": { "sharded": true, "foreignKeyMode": "FK_MANAGED", "vindexes": { @@ -694,8 +694,27 @@ "name": "hash_vin" } ] + }, + "tbl10": { + "column_vindexes": [ + { + "column": "sk", + "name": "hash_vin" + } + ] } } + }, + "unsharded_fk_allow": { + "foreignKeyMode": "FK_MANAGED", + "tables": { + "u_tbl1": {}, + "u_tbl2": {}, + "u_tbl3": {}, + "u_tbl4": {}, + "u_tbl5": {}, + "u_tbl6": {} + } } } } diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index 7813acb660f..052a204cc1b 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -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" @@ -54,9 +55,11 @@ func gen4UpdateStmtPlanner( } if ks, tables := semTable.SingleUnshardedKeyspace(); ks != nil { - plan := updateUnshardedShortcut(updStmt, ks, tables) - plan = pushCommentDirectivesOnPlan(plan, updStmt) - return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil + if fkManagementNotRequiredForUpdate(semTable, vschema, tables, updStmt.Exprs) { + plan := updateUnshardedShortcut(updStmt, ks, tables) + plan = pushCommentDirectivesOnPlan(plan, updStmt) + return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil + } } if semTable.NotUnshardedErr != nil { @@ -91,6 +94,38 @@ func gen4UpdateStmtPlanner( return newPlanResult(plan.Primitive(), operators.TablesUsed(op)...), nil } +// TODO: Handle all this in semantic analysis. +func fkManagementNotRequiredForUpdate(semTable *semantics.SemTable, vschema plancontext.VSchema, vTables []*vindexes.Table, updateExprs sqlparser.UpdateExprs) bool { + childFkMap := make(map[string][]vindexes.ChildFKInfo) + + // 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(vindexes.UpdateAction) + if len(childFks) > 0 { + childFkMap[vTable.String()] = childFks + } + } + + getFKInfo := func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { + tblInfo, err := semTable.TableInfoForExpr(expr.Name) + if err != nil { + return nil, nil + } + vTable := tblInfo.GetVindexTable() + return vTable.ParentForeignKeys, childFkMap[vTable.String()] + } + + // Check if any column in the parent table is being updated which has a child foreign key. + return !operators.ColumnModified(updateExprs, getFKInfo) +} + func updateUnshardedShortcut(stmt *sqlparser.Update, ks *vindexes.Keyspace, tables []*vindexes.Table) logicalPlan { edml := engine.NewDML() edml.Keyspace = ks diff --git a/go/vt/vtgate/vindexes/foreign_keys.go b/go/vt/vtgate/vindexes/foreign_keys.go index e18ed27ff25..5029c1f7fe1 100644 --- a/go/vt/vtgate/vindexes/foreign_keys.go +++ b/go/vt/vtgate/vindexes/foreign_keys.go @@ -107,16 +107,13 @@ func (vschema *VSchema) AddForeignKey(ksname, childTableName string, fkConstrain return nil } -// CrossShardParentFKs returns all the parent fk constraints on this table that are not shard scoped. -func (t *Table) CrossShardParentFKs() (crossShardFKs []ParentFKInfo) { - if len(t.ParentForeignKeys) == 0 { - return - } +// ParentFKsNeedsHandling returns all the parent fk constraints on this table that are not shard scoped. +func (t *Table) ParentFKsNeedsHandling() (fks []ParentFKInfo) { for _, fk := range t.ParentForeignKeys { // If the keyspaces are different, then the fk definition // is going to go across shards. if fk.Table.Keyspace.Name != t.Keyspace.Name { - crossShardFKs = append(crossShardFKs, fk) + fks = append(fks, fk) continue } // If the keyspaces match and they are unsharded, then the fk defintion @@ -126,7 +123,7 @@ func (t *Table) CrossShardParentFKs() (crossShardFKs []ParentFKInfo) { } if !isShardScoped(fk.Table, t, fk.ParentColumns, fk.ChildColumns) { - crossShardFKs = append(crossShardFKs, fk) + fks = append(fks, fk) } } return @@ -134,7 +131,7 @@ func (t *Table) CrossShardParentFKs() (crossShardFKs []ParentFKInfo) { // ChildFKsNeedsHandling retuns the child foreign keys that needs to be handled by the vtgate. // This can be either the foreign key is not shard scoped or the child tables needs cascading. -func (t *Table) ChildFKsNeedsHandling() (fks []ChildFKInfo) { +func (t *Table) ChildFKsNeedsHandling(getAction func(fk ChildFKInfo) sqlparser.ReferenceAction) (fks []ChildFKInfo) { for _, fk := range t.ChildForeignKeys { // If the keyspaces are different, then the fk definition // is going to go across shards. @@ -143,7 +140,7 @@ func (t *Table) ChildFKsNeedsHandling() (fks []ChildFKInfo) { continue } // If the action is not Restrict, then it needs a cascade. - switch fk.OnDelete { + switch getAction(fk) { case sqlparser.Cascade, sqlparser.SetNull, sqlparser.SetDefault: fks = append(fks, fk) continue @@ -159,6 +156,9 @@ func (t *Table) ChildFKsNeedsHandling() (fks []ChildFKInfo) { return } +func UpdateAction(fk ChildFKInfo) sqlparser.ReferenceAction { return fk.OnUpdate } +func DeleteAction(fk ChildFKInfo) sqlparser.ReferenceAction { return fk.OnDelete } + func isShardScoped(pTable *Table, cTable *Table, pCols sqlparser.Columns, cCols sqlparser.Columns) bool { if !pTable.Keyspace.Sharded { return true diff --git a/go/vt/vtgate/vindexes/foreign_keys_test.go b/go/vt/vtgate/vindexes/foreign_keys_test.go index 8a21e6909d0..b5dfa57fbce 100644 --- a/go/vt/vtgate/vindexes/foreign_keys_test.go +++ b/go/vt/vtgate/vindexes/foreign_keys_test.go @@ -137,7 +137,7 @@ func TestTable_CrossShardParentFKs(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - crossShardFks := tt.table.CrossShardParentFKs() + crossShardFks := tt.table.ParentFKsNeedsHandling() var crossShardFkTables []string for _, fk := range crossShardFks { crossShardFkTables = append(crossShardFkTables, fk.Table.Name.String()) @@ -243,9 +243,10 @@ func TestChildFKs(t *testing.T) { }, expChildTbls: []string{"t1"}, }} + deleteAction := func(fk ChildFKInfo) sqlparser.ReferenceAction { return fk.OnDelete } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - childFks := tt.table.ChildFKsNeedsHandling() + childFks := tt.table.ChildFKsNeedsHandling(deleteAction) var actualChildTbls []string for _, fk := range childFks { actualChildTbls = append(actualChildTbls, fk.Table.Name.String())