From a44d79971c9a391c0b6b7f59bcf416861bedbf81 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 16 Oct 2023 14:05:34 +0530 Subject: [PATCH] Refactor: foreign key in semantic analysis phase (#14273) Signed-off-by: Harshit Gangal Signed-off-by: Manan Gupta Co-authored-by: Manan Gupta --- go/vt/schemadiff/semantics.go | 5 + go/vt/vtgate/planbuilder/delete.go | 27 +- go/vt/vtgate/planbuilder/insert.go | 35 +- .../planbuilder/operators/ast2op_test.go | 205 ------ .../vtgate/planbuilder/operators/ast_to_op.go | 37 +- go/vt/vtgate/planbuilder/operators/delete.go | 14 +- go/vt/vtgate/planbuilder/operators/insert.go | 14 +- go/vt/vtgate/planbuilder/operators/update.go | 74 +- .../plancontext/planning_context.go | 5 - go/vt/vtgate/planbuilder/update.go | 61 +- go/vt/vtgate/semantics/FakeSI.go | 21 +- go/vt/vtgate/semantics/analyzer.go | 207 +++++- go/vt/vtgate/semantics/analyzer_test.go | 532 ++++++++++++++ go/vt/vtgate/semantics/info_schema.go | 5 + go/vt/vtgate/semantics/semantic_state.go | 167 +++++ go/vt/vtgate/semantics/semantic_state_test.go | 673 ++++++++++++++++++ go/vt/vtgate/vindexes/foreign_keys.go | 106 --- go/vt/vtgate/vindexes/foreign_keys_test.go | 314 -------- 18 files changed, 1647 insertions(+), 855 deletions(-) delete mode 100644 go/vt/vtgate/planbuilder/operators/ast2op_test.go delete mode 100644 go/vt/vtgate/vindexes/foreign_keys_test.go diff --git a/go/vt/schemadiff/semantics.go b/go/vt/schemadiff/semantics.go index ef9017d3b25..7ffb09007f5 100644 --- a/go/vt/schemadiff/semantics.go +++ b/go/vt/schemadiff/semantics.go @@ -20,6 +20,7 @@ import ( "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/vt/key" topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vschemapb "vitess.io/vitess/go/vt/proto/vschema" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/semantics" "vitess.io/vitess/go/vt/vtgate/vindexes" @@ -55,6 +56,10 @@ func (si *declarativeSchemaInformation) ConnCollation() collations.ID { return 45 } +func (si *declarativeSchemaInformation) ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyMode, error) { + return vschemapb.Keyspace_FK_UNMANAGED, nil +} + // 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/delete.go b/go/vt/vtgate/planbuilder/delete.go index b62c35e9bd1..385b8b7f924 100644 --- a/go/vt/vtgate/planbuilder/delete.go +++ b/go/vt/vtgate/planbuilder/delete.go @@ -18,7 +18,6 @@ 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" @@ -56,8 +55,14 @@ func gen4DeleteStmtPlanner( return nil, err } + // Remove all the foreign keys that don't require any handling. + err = ctx.SemTable.RemoveNonRequiredForeignKeys(ctx.VerifyAllFKs, vindexes.DeleteAction) + if err != nil { + return nil, err + } + if ks, tables := ctx.SemTable.SingleUnshardedKeyspace(); ks != nil { - if fkManagementNotRequired(ctx, vschema, tables) { + if !ctx.SemTable.ForeignKeysPresent() { plan := deleteUnshardedShortcut(deleteStmt, ks, tables) return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil } @@ -93,24 +98,6 @@ func gen4DeleteStmtPlanner( return newPlanResult(plan.Primitive(), operators.TablesUsed(op)...), nil } -func fkManagementNotRequired(ctx *plancontext.PlanningContext, 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(ctx.VerifyAllFKs, vindexes.DeleteAction) - 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 { diff --git a/go/vt/vtgate/planbuilder/insert.go b/go/vt/vtgate/planbuilder/insert.go index 55ef9148b1a..8eba43ce471 100644 --- a/go/vt/vtgate/planbuilder/insert.go +++ b/go/vt/vtgate/planbuilder/insert.go @@ -18,7 +18,6 @@ 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" @@ -45,11 +44,13 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm // Check single unsharded. Even if the table is for single unsharded but sequence table is used. // We cannot shortcut here as sequence column needs additional planning. ks, tables := ctx.SemTable.SingleUnshardedKeyspace() - fkPlanNeeded := false + // Remove all the foreign keys that don't require any handling. + err = ctx.SemTable.RemoveNonRequiredForeignKeys(ctx.VerifyAllFKs, vindexes.UpdateAction) + if err != nil { + return nil, err + } if ks != nil { - noAutoInc := tables[0].AutoIncrement == nil - fkPlanNeeded = fkManagementRequiredForInsert(ctx, tables[0], sqlparser.UpdateExprs(insStmt.OnDup), insStmt.Action == sqlparser.ReplaceAct) - if noAutoInc && !fkPlanNeeded { + if tables[0].AutoIncrement == nil && !ctx.SemTable.ForeignKeysPresent() { plan := insertUnshardedShortcut(insStmt, ks, tables) plan = pushCommentDirectivesOnPlan(plan, insStmt) return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil @@ -61,7 +62,7 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm return nil, err } - if err = errOutIfPlanCannotBeConstructed(ctx, tblInfo.GetVindexTable(), insStmt, fkPlanNeeded); err != nil { + if err = errOutIfPlanCannotBeConstructed(ctx, tblInfo.GetVindexTable(), insStmt, ctx.SemTable.ForeignKeysPresent()); err != nil { return nil, err } @@ -104,28 +105,6 @@ func errOutIfPlanCannotBeConstructed(ctx *plancontext.PlanningContext, vTbl *vin return nil } -// TODO: Handle all this in semantic analysis. -func fkManagementRequiredForInsert(ctx *plancontext.PlanningContext, vTbl *vindexes.Table, updateExprs sqlparser.UpdateExprs, replace bool) bool { - ksMode, err := ctx.VSchema.ForeignKeyMode(vTbl.Keyspace.Name) - if err != nil || ksMode != vschemapb.Keyspace_FK_MANAGED { - return false - } - - if len(vTbl.ParentFKsNeedsHandling(ctx.VerifyAllFKs, "")) > 0 { - return true - } - - childFks := vTbl.ChildFKsNeedsHandling(ctx.VerifyAllFKs, vindexes.UpdateAction) - if len(childFks) > 0 && replace { - return true - } - - // Check if any column in the parent table is being updated which has a child foreign key. - return columnModified(updateExprs, func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { - return nil, childFks - }) -} - func insertUnshardedShortcut(stmt *sqlparser.Insert, ks *vindexes.Keyspace, tables []*vindexes.Table) logicalPlan { eIns := &engine.Insert{} eIns.Keyspace = ks diff --git a/go/vt/vtgate/planbuilder/operators/ast2op_test.go b/go/vt/vtgate/planbuilder/operators/ast2op_test.go deleted file mode 100644 index 4dbcf49e80a..00000000000 --- a/go/vt/vtgate/planbuilder/operators/ast2op_test.go +++ /dev/null @@ -1,205 +0,0 @@ -/* -Copyright 2023 The Vitess Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package operators - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" - "vitess.io/vitess/go/vt/vtgate/vindexes" -) - -// Test_fkNeedsHandlingForUpdates tests the functionality of the function fkNeedsHandlingForUpdates. -// It verifies the different cases in which foreign key handling is required on vtgate level. -func Test_fkNeedsHandlingForUpdates(t *testing.T) { - t1 := &vindexes.Table{ - Name: sqlparser.NewIdentifierCS("t1"), - Keyspace: &vindexes.Keyspace{Name: "ks"}, - } - t2 := &vindexes.Table{ - Name: sqlparser.NewIdentifierCS("t2"), - Keyspace: &vindexes.Keyspace{Name: "ks2"}, - } - t3 := &vindexes.Table{ - Name: sqlparser.NewIdentifierCS("t3"), - Keyspace: &vindexes.Keyspace{Name: "ks"}, - } - - tests := []struct { - name string - verifyAllFks bool - parentFkToIgnore string - updateExprs sqlparser.UpdateExprs - parentFks []vindexes.ParentFKInfo - childFks []vindexes.ChildFKInfo - parentFKsWanted []bool - childFKsWanted []bool - }{{ - name: "No Fks filtered", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFKsWanted: []bool{true}, - childFKsWanted: []bool{true}, - }, { - name: "Child Fks filtering", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("b", "a", "c")}, - {Table: t2, ParentColumns: sqlparser.MakeColumns("d", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFKsWanted: []bool{true}, - childFKsWanted: []bool{true, false}, - }, { - name: "Parent Fks filtered based on columns", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, - {Table: t2, ChildColumns: sqlparser.MakeColumns("d", "b")}, - }, - parentFKsWanted: []bool{true, false}, - childFKsWanted: []bool{true}, - }, { - name: "Parent Fks filtered because all null values", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: &sqlparser.NullVal{}}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b")}, - }, - parentFKsWanted: []bool{false, false}, - childFKsWanted: []bool{true}, - }, { - name: "Parent Fks filtered because some column has null values", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("c"), Expr: &sqlparser.NullVal{}}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b")}, - {Table: t3, ChildColumns: sqlparser.MakeColumns("a", "b")}, - }, - parentFKsWanted: []bool{false, true, false}, - childFKsWanted: []bool{true}, - }, { - name: "Unsharded fk with verifyAllFk", - verifyAllFks: true, - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("c"), Expr: &sqlparser.NullVal{}}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b")}, - {Table: t3, ChildColumns: sqlparser.MakeColumns("a", "b")}, - {Table: t3, ChildColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFKsWanted: []bool{false, true, true, false}, - childFKsWanted: []bool{true}, - }, { - name: "Mixed case", - verifyAllFks: true, - parentFkToIgnore: "ks.t1abks.t3", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("c"), Expr: &sqlparser.NullVal{}}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b")}, - {Table: t3, ChildColumns: sqlparser.MakeColumns("a", "b")}, - {Table: t3, ChildColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFKsWanted: []bool{false, true, false, false}, - childFKsWanted: []bool{true}, - }, { - name: "Ignore Fk specified", - parentFkToIgnore: "ks.t1aefks2.t2", - updateExprs: sqlparser.UpdateExprs{ - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("a"), Expr: sqlparser.NewIntLiteral("1")}, - &sqlparser.UpdateExpr{Name: sqlparser.NewColName("c"), Expr: &sqlparser.NullVal{}}, - }, - childFks: []vindexes.ChildFKInfo{ - {Table: t2, ParentColumns: sqlparser.MakeColumns("a", "b", "c")}, - }, - parentFks: []vindexes.ParentFKInfo{ - {Table: t2, ChildColumns: sqlparser.MakeColumns("b", "a", "c")}, - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "b")}, - {Table: t2, ChildColumns: sqlparser.MakeColumns("a", "e", "f")}, - }, - parentFKsWanted: []bool{false, true, false}, - childFKsWanted: []bool{true}, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t1.ParentForeignKeys = tt.parentFks - t1.ChildForeignKeys = tt.childFks - ctx := &plancontext.PlanningContext{ - VerifyAllFKs: tt.verifyAllFks, - ParentFKToIgnore: tt.parentFkToIgnore, - } - parentFksGot, childFksGot := getFKRequirementsForUpdate(ctx, tt.updateExprs, t1) - var pFks []vindexes.ParentFKInfo - for idx, expected := range tt.parentFKsWanted { - if expected { - pFks = append(pFks, tt.parentFks[idx]) - } - } - var cFks []vindexes.ChildFKInfo - for idx, expected := range tt.childFKsWanted { - if expected { - cFks = append(cFks, tt.childFks[idx]) - } - } - require.EqualValues(t, pFks, parentFksGot) - require.EqualValues(t, cFks, childFksGot) - }) - } -} diff --git a/go/vt/vtgate/planbuilder/operators/ast_to_op.go b/go/vt/vtgate/planbuilder/operators/ast_to_op.go index af546bc8b85..83f5d268094 100644 --- a/go/vt/vtgate/planbuilder/operators/ast_to_op.go +++ b/go/vt/vtgate/planbuilder/operators/ast_to_op.go @@ -199,18 +199,43 @@ func createOperatorFromUnion(ctx *plancontext.PlanningContext, node *sqlparser.U } // createOpFromStmt creates an operator from the given statement. It takes in two additional arguments— -// 1. verifyAllFKs: For this given statement, do we need to verify validity of all the foreign keys on the vtgate level. -// 2. fkToIgnore: The foreign key constraint to specifically ignore while planning the statement. +// 1. verifyAllFKs: For this given statement, do we need to verify validity of all the foreign keys on the vtgate level. +// 2. fkToIgnore: The foreign key constraint to specifically ignore while planning the statement. This field is used in UPDATE CASCADE planning, wherein while planning the child update +// query, we need to ignore the parent foreign key constraint that caused the cascade in question. func createOpFromStmt(ctx *plancontext.PlanningContext, stmt sqlparser.Statement, verifyAllFKs bool, fkToIgnore string) (ops.Operator, error) { - newCtx, err := plancontext.CreatePlanningContext(stmt, ctx.ReservedVars, ctx.VSchema, ctx.PlannerVersion) + var err error + ctx, err = plancontext.CreatePlanningContext(stmt, ctx.ReservedVars, ctx.VSchema, ctx.PlannerVersion) if err != nil { return nil, err } - newCtx.VerifyAllFKs = verifyAllFKs - newCtx.ParentFKToIgnore = fkToIgnore + // TODO (@GuptaManan100, @harshit-gangal): When we add cross-shard foreign keys support, + // we should augment the semantic analysis to also tell us whether the given query has any cross shard parent foreign keys to validate. + // If there are, then we have to run the query with FOREIGN_KEY_CHECKS off because we can't be sure if the DML will succeed on MySQL with the checks on. + // So, we should set VerifyAllFKs to true. i.e. we should add `|| ctx.SemTable.RequireForeignKeyChecksOff()` to the below condition. + ctx.VerifyAllFKs = verifyAllFKs - return PlanQuery(newCtx, stmt) + // From all the parent foreign keys involved, we should remove the one that we need to ignore. + err = ctx.SemTable.RemoveParentForeignKey(fkToIgnore) + if err != nil { + return nil, err + } + + // Now, we can filter the foreign keys further based on the planning context, specifically whether we are running + // this query with FOREIGN_KEY_CHECKS off or not. If the foreign key checks are enabled, then we don't need to verify + // the validity of shard-scoped RESTRICT foreign keys, since MySQL will do that for us. Similarily, we don't need to verify + // if the shard-scoped parent foreign key constraints are valid. + switch stmt.(type) { + case *sqlparser.Update, *sqlparser.Insert: + err = ctx.SemTable.RemoveNonRequiredForeignKeys(ctx.VerifyAllFKs, vindexes.UpdateAction) + case *sqlparser.Delete: + err = ctx.SemTable.RemoveNonRequiredForeignKeys(ctx.VerifyAllFKs, vindexes.DeleteAction) + } + if err != nil { + return nil, err + } + + return PlanQuery(ctx, stmt) } func getOperatorFromTableExpr(ctx *plancontext.PlanningContext, tableExpr sqlparser.TableExpr, onlyTable bool) (ops.Operator, error) { diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index 2455bb958b5..128c625c428 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -19,7 +19,6 @@ package operators import ( "fmt" - 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" @@ -92,18 +91,7 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp return nil, err } - // Now we check for the foreign key mode and make changes if required. - ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) - if err != nil { - return nil, err - } - - // Unmanaged foreign-key-mode, we don't need to do anything. - if ksMode != vschemapb.Keyspace_FK_MANAGED { - return delOp, nil - } - - childFks := vindexTable.ChildFKsNeedsHandling(ctx.VerifyAllFKs, vindexes.DeleteAction) + childFks := ctx.SemTable.GetChildForeignKeysList() // If there are no foreign key constraints, then we don't need to do anything. if len(childFks) == 0 { return delOp, nil diff --git a/go/vt/vtgate/planbuilder/operators/insert.go b/go/vt/vtgate/planbuilder/operators/insert.go index 8bdee0a11a8..57c01da5d4b 100644 --- a/go/vt/vtgate/planbuilder/operators/insert.go +++ b/go/vt/vtgate/planbuilder/operators/insert.go @@ -153,17 +153,13 @@ func createOperatorFromInsert(ctx *plancontext.PlanningContext, ins *sqlparser.I return insOp, nil } - parentFKsForInsert := vindexTable.ParentFKsNeedsHandling(ctx.VerifyAllFKs, ctx.ParentFKToIgnore) - if len(parentFKsForInsert) > 0 { - return nil, vterrors.VT12002() - } - if len(ins.OnDup) == 0 { + parentFKs := ctx.SemTable.GetParentForeignKeysList() + childFks := ctx.SemTable.GetChildForeignKeysList() + if len(childFks) == 0 && len(parentFKs) == 0 { return insOp, nil } - - parentFksForUpdate, childFksForUpdate := getFKRequirementsForUpdate(ctx, sqlparser.UpdateExprs(ins.OnDup), vindexTable) - if len(parentFksForUpdate) == 0 && len(childFksForUpdate) == 0 { - return insOp, nil + if len(parentFKs) > 0 { + return nil, vterrors.VT12002() } return nil, vterrors.VT12001("ON DUPLICATE KEY UPDATE with foreign keys") } diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index c76baabe7dd..252e5dd6195 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -22,7 +22,6 @@ import ( "slices" "strings" - 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" @@ -115,16 +114,8 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars return nil, err } - ksMode, err := ctx.VSchema.ForeignKeyMode(vindexTable.Keyspace.Name) - if err != nil { - return nil, err - } - // Unmanaged foreign-key-mode, we don't need to do anything. - if ksMode != vschemapb.Keyspace_FK_MANAGED { - return updOp, nil - } - - parentFks, childFks := getFKRequirementsForUpdate(ctx, updStmt.Exprs, vindexTable) + parentFks := ctx.SemTable.GetParentForeignKeysList() + childFks := ctx.SemTable.GetChildForeignKeysList() if len(childFks) == 0 && len(parentFks) == 0 { return updOp, nil } @@ -204,67 +195,6 @@ func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.U return sqc.getRootOperator(route), nil } -// getFKRequirementsForUpdate analyzes update expressions to determine which foreign key constraints needs management at the VTGate. -// It identifies parent and child foreign keys that require verification or cascade operations due to column updates. -func getFKRequirementsForUpdate(ctx *plancontext.PlanningContext, updateExprs sqlparser.UpdateExprs, vindexTable *vindexes.Table) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { - parentFks := vindexTable.ParentFKsNeedsHandling(ctx.VerifyAllFKs, ctx.ParentFKToIgnore) - childFks := vindexTable.ChildFKsNeedsHandling(ctx.VerifyAllFKs, vindexes.UpdateAction) - if len(childFks) == 0 && len(parentFks) == 0 { - return nil, nil - } - - pFksRequired := make([]bool, len(parentFks)) - cFksRequired := make([]bool, len(childFks)) - // Go over all the update expressions - for _, updateExpr := range updateExprs { - // Any foreign key to a child table for a column that has been updated - // will require the cascade operations or restrict verification to happen, so we include all such foreign keys. - for idx, childFk := range childFks { - if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { - cFksRequired[idx] = true - } - } - // If we are setting a column to NULL, then we don't need to verify the existance of an - // equivalent row in the parent table, even if this column was part of a foreign key to a parent table. - if sqlparser.IsNull(updateExpr.Expr) { - continue - } - // We add all the possible parent foreign key constraints that need verification that an equivalent row - // exists, given that this column has changed. - for idx, parentFk := range parentFks { - if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequired[idx] = true - } - } - } - // For the parent foreign keys, if any of the columns part of the fk is set to NULL, - // then, we don't care for the existance of an equivalent row in the parent table. - for idx, parentFk := range parentFks { - for _, updateExpr := range updateExprs { - if !sqlparser.IsNull(updateExpr.Expr) { - continue - } - if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - pFksRequired[idx] = false - } - } - } - // Get the filtered lists and return them. - var pFksNeedsHandling []vindexes.ParentFKInfo - var cFksNeedsHandling []vindexes.ChildFKInfo - for idx, parentFk := range parentFks { - if pFksRequired[idx] { - pFksNeedsHandling = append(pFksNeedsHandling, parentFk) - } - } - for idx, childFk := range childFks { - if cFksRequired[idx] { - cFksNeedsHandling = append(cFksNeedsHandling, childFk) - } - } - return pFksNeedsHandling, cFksNeedsHandling -} - func buildFkOperator(ctx *plancontext.PlanningContext, updOp ops.Operator, updClone *sqlparser.Update, parentFks []vindexes.ParentFKInfo, childFks []vindexes.ChildFKInfo, updatedTable *vindexes.Table) (ops.Operator, error) { // We only support simple expressions in update queries for foreign key handling. if isNonLiteral(updClone.Exprs, parentFks, childFks) { diff --git a/go/vt/vtgate/planbuilder/plancontext/planning_context.go b/go/vt/vtgate/planbuilder/plancontext/planning_context.go index d090a593a39..68ccc95b9fd 100644 --- a/go/vt/vtgate/planbuilder/plancontext/planning_context.go +++ b/go/vt/vtgate/planbuilder/plancontext/planning_context.go @@ -43,11 +43,6 @@ type PlanningContext struct { // This is required for queries we are running with /*+ SET_VAR(foreign_key_checks=OFF) */ VerifyAllFKs bool - // ParentFKToIgnore stores a specific parent foreign key that we would need to ignore while planning - // a certain query. This field is used in UPDATE CASCADE planning, wherein while planning the child update - // query, we need to ignore the parent foreign key constraint that caused the cascade in question. - ParentFKToIgnore string - // Projected subqueries that have been merged MergedSubqueries []*sqlparser.Subquery diff --git a/go/vt/vtgate/planbuilder/update.go b/go/vt/vtgate/planbuilder/update.go index b99631f6b55..6b2d492c1fa 100644 --- a/go/vt/vtgate/planbuilder/update.go +++ b/go/vt/vtgate/planbuilder/update.go @@ -18,7 +18,6 @@ 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" @@ -47,8 +46,13 @@ func gen4UpdateStmtPlanner( return nil, err } + // Remove all the foreign keys that don't require any handling. + err = ctx.SemTable.RemoveNonRequiredForeignKeys(ctx.VerifyAllFKs, vindexes.UpdateAction) + if err != nil { + return nil, err + } if ks, tables := ctx.SemTable.SingleUnshardedKeyspace(); ks != nil { - if fkManagementNotRequiredForUpdate(ctx, tables, updStmt.Exprs) { + if !ctx.SemTable.ForeignKeysPresent() { plan := updateUnshardedShortcut(updStmt, ks, tables) plan = pushCommentDirectivesOnPlan(plan, updStmt) return newPlanResult(plan.Primitive(), operators.QualifiedTables(ks, tables)...), nil @@ -85,59 +89,6 @@ func gen4UpdateStmtPlanner( return newPlanResult(plan.Primitive(), operators.TablesUsed(op)...), nil } -// TODO: Handle all this in semantic analysis. -func fkManagementNotRequiredForUpdate(ctx *plancontext.PlanningContext, 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 := ctx.VSchema.ForeignKeyMode(vTable.Keyspace.Name) - if err != nil { - return false - } - if ksMode != vschemapb.Keyspace_FK_MANAGED { - continue - } - childFks := vTable.ChildFKsNeedsHandling(ctx.VerifyAllFKs, vindexes.UpdateAction) - if len(childFks) > 0 { - childFkMap[vTable.String()] = childFks - } - } - - getFKInfo := func(expr *sqlparser.UpdateExpr) ([]vindexes.ParentFKInfo, []vindexes.ChildFKInfo) { - tblInfo, err := ctx.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 !columnModified(updateExprs, getFKInfo) -} - -// 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 - } - } - if sqlparser.IsNull(updateExpr.Expr) { - continue - } - for _, parentFk := range parentFKs { - if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { - return true - } - } - } - return false -} - func updateUnshardedShortcut(stmt *sqlparser.Update, ks *vindexes.Keyspace, tables []*vindexes.Table) logicalPlan { edml := engine.NewDML() edml.Keyspace = ks diff --git a/go/vt/vtgate/semantics/FakeSI.go b/go/vt/vtgate/semantics/FakeSI.go index 1c2071f26ce..580ff346d8b 100644 --- a/go/vt/vtgate/semantics/FakeSI.go +++ b/go/vt/vtgate/semantics/FakeSI.go @@ -17,9 +17,12 @@ limitations under the License. package semantics import ( + "fmt" + "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/vt/key" topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vschemapb "vitess.io/vitess/go/vt/proto/vschema" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/vindexes" ) @@ -28,8 +31,9 @@ var _ SchemaInformation = (*FakeSI)(nil) // FakeSI is a fake SchemaInformation for testing type FakeSI struct { - Tables map[string]*vindexes.Table - VindexTables map[string]vindexes.Vindex + Tables map[string]*vindexes.Table + VindexTables map[string]vindexes.Vindex + KsForeignKeyMode map[string]vschemapb.Keyspace_ForeignKeyMode } // FindTableOrVindex implements the SchemaInformation interface @@ -41,6 +45,17 @@ func (s *FakeSI) FindTableOrVindex(tablename sqlparser.TableName) (*vindexes.Tab return nil, s.VindexTables[sqlparser.String(tablename)], "", 0, nil, nil } -func (FakeSI) ConnCollation() collations.ID { +func (*FakeSI) ConnCollation() collations.ID { return 45 } + +func (s *FakeSI) ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyMode, error) { + if s.KsForeignKeyMode != nil { + fkMode, isPresent := s.KsForeignKeyMode[keyspace] + if !isPresent { + return vschemapb.Keyspace_FK_DEFAULT, fmt.Errorf("%v keyspace not found", keyspace) + } + return fkMode, nil + } + return vschemapb.Keyspace_FK_UNMANAGED, nil +} diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index 1cb457f6882..aaf33e98a8b 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -18,8 +18,10 @@ package semantics import ( "vitess.io/vitess/go/mysql/collations" + 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/vindexes" ) // analyzer controls the flow of the analysis. @@ -74,9 +76,7 @@ func Analyze(statement sqlparser.Statement, currentDb string, si SchemaInformati } // Creation of the semantic table - semTable := analyzer.newSemTable(statement, si.ConnCollation()) - - return semTable, nil + return analyzer.newSemTable(statement, si.ConnCollation()) } // AnalyzeStrict analyzes the parsed query, and fails the analysis for any possible errors @@ -96,7 +96,7 @@ func AnalyzeStrict(statement sqlparser.Statement, currentDb string, si SchemaInf return st, nil } -func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID) *SemTable { +func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID) (*SemTable, error) { var comments *sqlparser.ParsedComments commentedStmt, isCommented := statement.(sqlparser.Commented) if isCommented { @@ -107,22 +107,29 @@ func (a *analyzer) newSemTable(statement sqlparser.Statement, coll collations.ID columns[union] = info.exprs } - return &SemTable{ - Recursive: a.binder.recursive, - Direct: a.binder.direct, - ExprTypes: a.typer.exprTypes, - Tables: a.tables.Tables, - NotSingleRouteErr: a.projErr, - NotUnshardedErr: a.unshardedErr, - Warning: a.warning, - Comments: comments, - ColumnEqualities: map[columnName][]sqlparser.Expr{}, - Collation: coll, - ExpandedColumns: a.rewriter.expandedColumns, - columns: columns, - StatementIDs: a.scoper.statementIDs, - QuerySignature: a.sig, + childFks, parentFks, err := a.getInvolvedForeignKeys(statement) + if err != nil { + return nil, err } + + return &SemTable{ + Recursive: a.binder.recursive, + Direct: a.binder.direct, + ExprTypes: a.typer.exprTypes, + Tables: a.tables.Tables, + NotSingleRouteErr: a.projErr, + NotUnshardedErr: a.unshardedErr, + Warning: a.warning, + Comments: comments, + ColumnEqualities: map[columnName][]sqlparser.Expr{}, + Collation: coll, + ExpandedColumns: a.rewriter.expandedColumns, + columns: columns, + StatementIDs: a.scoper.statementIDs, + QuerySignature: a.sig, + childForeignKeysInvolved: childFks, + parentForeignKeysInvolved: parentFks, + }, nil } func (a *analyzer) setError(err error) { @@ -312,6 +319,168 @@ 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) { + // There are only the DML statements that require any foreign keys handling. + switch stmt := statement.(type) { + case *sqlparser.Delete: + // For DELETE statements, none of the parent foreign keys require handling. + // So we collect all the child foreign keys. + allChildFks, _, err := a.getAllManagedForeignKeys() + return allChildFks, nil, err + case *sqlparser.Insert: + // For INSERT statements, we have 3 different cases: + // 1. REPLACE statement: REPLACE statements are essentially DELETEs and INSERTs rolled into one. + // So we need to the parent foreign keys to ensure we are inserting the correct values, and the child foreign keys + // to ensure we don't change a row that breaks the constraint or cascade any operations on the child tables. + // 2. Normal INSERT statement: We don't need to check anything on the child foreign keys, so we just get all the parent foreign keys. + // 3. INSERT with ON DUPLICATE KEY UPDATE: This might trigger an update on the columns specified in the ON DUPLICATE KEY UPDATE clause. + allChildFks, allParentFKs, err := a.getAllManagedForeignKeys() + if err != nil { + return nil, nil, err + } + if stmt.Action == sqlparser.ReplaceAct { + return allChildFks, allParentFKs, nil + } + if len(stmt.OnDup) == 0 { + return nil, allParentFKs, nil + } + // If only a certain set of columns are being updated, then there might be some child foreign keys that don't need any consideration since their columns aren't being updated. + // So, we filter these child foreign keys out. We can't filter any parent foreign keys because the statement will INSERT a row too, which requires validating all the parent foreign keys. + updatedChildFks, _ := a.filterForeignKeysUsingUpdateExpressions(allChildFks, nil, sqlparser.UpdateExprs(stmt.OnDup)) + return updatedChildFks, allParentFKs, nil + case *sqlparser.Update: + // For UPDATE queries we get all the parent and child foreign keys, but we can filter some of them out if the columns that they consist off aren't being updated or are set to NULLs. + allChildFks, allParentFks, err := a.getAllManagedForeignKeys() + if err != nil { + return nil, nil, err + } + childFks, parentFks := a.filterForeignKeysUsingUpdateExpressions(allChildFks, allParentFks, stmt.Exprs) + return childFks, parentFks, nil + default: + return nil, nil, nil + } +} + +// filterForeignKeysUsingUpdateExpressions filters the child and parent foreign key constraints that don't require any validations/cascades given the updated expressions. +func (a *analyzer) filterForeignKeysUsingUpdateExpressions(allChildFks map[TableSet][]vindexes.ChildFKInfo, allParentFks map[TableSet][]vindexes.ParentFKInfo, updExprs sqlparser.UpdateExprs) (map[TableSet][]vindexes.ChildFKInfo, map[TableSet][]vindexes.ParentFKInfo) { + if len(allChildFks) == 0 && len(allParentFks) == 0 { + return nil, nil + } + + pFksRequired := make(map[TableSet][]bool, len(allParentFks)) + cFksRequired := make(map[TableSet][]bool, len(allChildFks)) + for ts, fks := range allParentFks { + pFksRequired[ts] = make([]bool, len(fks)) + } + for ts, fks := range allChildFks { + cFksRequired[ts] = make([]bool, len(fks)) + } + + // updExprToTableSet stores the tables that the updated expressions are from. + updExprToTableSet := make(map[*sqlparser.ColName]TableSet) + + // Go over all the update expressions + for _, updateExpr := range updExprs { + deps := a.binder.direct.dependencies(updateExpr.Name) + if deps.NumberOfTables() != 1 { + panic("expected to have single table dependency") + } + updExprToTableSet[updateExpr.Name] = deps + // Get all the child and parent foreign keys for the given table that the update expression belongs to. + childFks := allChildFks[deps] + parentFKs := allParentFks[deps] + + // Any foreign key to a child table for a column that has been updated + // will require the cascade operations or restrict verification to happen, so we include all such foreign keys. + for idx, childFk := range childFks { + if childFk.ParentColumns.FindColumn(updateExpr.Name.Name) >= 0 { + cFksRequired[deps][idx] = true + } + } + // If we are setting a column to NULL, then we don't need to verify the existance of an + // equivalent row in the parent table, even if this column was part of a foreign key to a parent table. + if sqlparser.IsNull(updateExpr.Expr) { + continue + } + // We add all the possible parent foreign key constraints that need verification that an equivalent row + // exists, given that this column has changed. + for idx, parentFk := range parentFKs { + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + pFksRequired[deps][idx] = true + } + } + } + // For the parent foreign keys, if any of the columns part of the fk is set to NULL, + // then, we don't care for the existence of an equivalent row in the parent table. + for _, updateExpr := range updExprs { + if !sqlparser.IsNull(updateExpr.Expr) { + continue + } + ts := updExprToTableSet[updateExpr.Name] + parentFKs := allParentFks[ts] + for idx, parentFk := range parentFKs { + if parentFk.ChildColumns.FindColumn(updateExpr.Name.Name) >= 0 { + pFksRequired[ts][idx] = false + } + } + } + + // Create new maps with only the required foreign keys. + pFksNeedsHandling := map[TableSet][]vindexes.ParentFKInfo{} + cFksNeedsHandling := map[TableSet][]vindexes.ChildFKInfo{} + for ts, parentFks := range allParentFks { + var pFKNeeded []vindexes.ParentFKInfo + for idx, fk := range parentFks { + if pFksRequired[ts][idx] { + pFKNeeded = append(pFKNeeded, fk) + } + } + pFksNeedsHandling[ts] = pFKNeeded + + } + for ts, childFks := range allChildFks { + var cFKNeeded []vindexes.ChildFKInfo + for idx, fk := range childFks { + if cFksRequired[ts][idx] { + cFKNeeded = append(cFKNeeded, fk) + } + } + cFksNeedsHandling[ts] = cFKNeeded + + } + return cFksNeedsHandling, pFksNeedsHandling +} + +// getAllManagedForeignKeys gets all the foreign keys for the query we are analyzing that Vitess is reposible for managing. +func (a *analyzer) getAllManagedForeignKeys() (map[TableSet][]vindexes.ChildFKInfo, map[TableSet][]vindexes.ParentFKInfo, error) { + allChildFKs := make(map[TableSet][]vindexes.ChildFKInfo) + allParentFKs := make(map[TableSet][]vindexes.ParentFKInfo) + + // Go over all the tables and collect the foreign keys. + for idx, table := range a.tables.Tables { + vi := table.GetVindexTable() + if vi == nil || vi.Keyspace == nil { + // If is not a real table, so should be skipped. + continue + } + // Check whether Vitess needs to manage the foreign keys in this keyspace or not. + fkMode, err := a.tables.si.ForeignKeyMode(vi.Keyspace.Name) + if err != nil { + return nil, nil, err + } + if fkMode != vschemapb.Keyspace_FK_MANAGED { + continue + } + + // Add all the child and parent foreign keys to our map. + ts := SingleTableSet(idx) + allChildFKs[ts] = vi.ChildForeignKeys + allParentFKs[ts] = vi.ParentForeignKeys + } + return allChildFKs, allParentFKs, nil +} + // ProjError is used to mark an error as something that should only be returned // if the planner fails to merge everything down to a single route type ProjError struct { diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go index 21222da2263..dd2d80d62d4 100644 --- a/go/vt/vtgate/semantics/analyzer_test.go +++ b/go/vt/vtgate/semantics/analyzer_test.go @@ -24,6 +24,7 @@ import ( "vitess.io/vitess/go/sqltypes" 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/vtgate/vindexes" ) @@ -1519,3 +1520,534 @@ func fakeSchemaInfo() *FakeSI { } return si } + +var tbl = map[string]TableInfo{ + "t0": &RealTable{ + Table: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + ChildForeignKeys: []vindexes.ChildFKInfo{ + ckInfo(nil, []string{"col"}, []string{"col"}, sqlparser.Restrict), + ckInfo(nil, []string{"col1", "col2"}, []string{"ccol1", "ccol2"}, sqlparser.SetNull), + }, + ParentForeignKeys: []vindexes.ParentFKInfo{ + pkInfo(nil, []string{"colb"}, []string{"colb"}), + pkInfo(nil, []string{"colb1", "colb2"}, []string{"ccolb1", "ccolb2"}), + }, + }, + }, + "t1": &RealTable{ + Table: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks_unmanaged", Sharded: true}, + ChildForeignKeys: []vindexes.ChildFKInfo{ + ckInfo(nil, []string{"cola"}, []string{"cola"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola1", "cola2"}, []string{"ccola1", "ccola2"}, sqlparser.SetNull), + }, + }, + }, + "t2": &RealTable{ + Table: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + }, + }, + "t3": &RealTable{ + Table: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "undefined_ks", Sharded: true}, + }, + }, +} + +// TestGetAllManagedForeignKeys tests the functionality of getAllManagedForeignKeys. +func TestGetAllManagedForeignKeys(t *testing.T) { + tests := []struct { + name string + analyzer *analyzer + childFkWanted map[TableSet][]vindexes.ChildFKInfo + parentFkWanted map[TableSet][]vindexes.ParentFKInfo + expectedErr string + }{ + { + name: "Collect all foreign key constraints", + analyzer: &analyzer{ + tables: &tableCollector{ + Tables: []TableInfo{tbl["t0"], tbl["t1"], + &DerivedTable{}, + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + "ks_unmanaged": vschemapb.Keyspace_FK_UNMANAGED, + }, + }, + }, + }, + childFkWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"col"}, []string{"col"}, sqlparser.Restrict), + ckInfo(nil, []string{"col1", "col2"}, []string{"ccol1", "ccol2"}, sqlparser.SetNull), + }, + }, + parentFkWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"colb"}, []string{"colb"}), + pkInfo(nil, []string{"colb1", "colb2"}, []string{"ccolb1", "ccolb2"}), + }, + }, + }, + { + name: "keyspace not found in schema information", + analyzer: &analyzer{ + tables: &tableCollector{ + Tables: []TableInfo{ + tbl["t2"], + tbl["t3"], + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + }, + }, + }, + }, + expectedErr: "undefined_ks keyspace not found", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + childFk, parentFk, err := tt.analyzer.getAllManagedForeignKeys() + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) + return + } + require.EqualValues(t, tt.childFkWanted, childFk) + require.EqualValues(t, tt.parentFkWanted, parentFk) + }) + } +} + +// TestFilterForeignKeysUsingUpdateExpressions tests the functionality of filterForeignKeysUsingUpdateExpressions. +func TestFilterForeignKeysUsingUpdateExpressions(t *testing.T) { + cola := sqlparser.NewColName("cola") + colb := sqlparser.NewColName("colb") + colc := sqlparser.NewColName("colc") + cold := sqlparser.NewColName("cold") + a := &analyzer{ + binder: &binder{ + direct: map[sqlparser.Expr]TableSet{ + cola: SingleTableSet(0), + colb: SingleTableSet(0), + colc: SingleTableSet(1), + cold: SingleTableSet(1), + }, + }, + } + updateExprs := sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{Name: cola, Expr: sqlparser.NewIntLiteral("1")}, + &sqlparser.UpdateExpr{Name: colb, Expr: &sqlparser.NullVal{}}, + &sqlparser.UpdateExpr{Name: colc, Expr: sqlparser.NewIntLiteral("1")}, + &sqlparser.UpdateExpr{Name: cold, Expr: &sqlparser.NullVal{}}, + } + tests := []struct { + name string + analyzer *analyzer + allChildFks map[TableSet][]vindexes.ChildFKInfo + allParentFks map[TableSet][]vindexes.ParentFKInfo + updExprs sqlparser.UpdateExprs + childFksWanted map[TableSet][]vindexes.ChildFKInfo + parentFksWanted map[TableSet][]vindexes.ParentFKInfo + }{ + { + name: "Child Foreign Keys Filtering", + analyzer: a, + allParentFks: nil, + allChildFks: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + }, + SingleTableSet(1): { + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + ckInfo(nil, []string{"colc", "colx"}, []string{"child_colc", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + }, + }, + updExprs: updateExprs, + childFksWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + }, + SingleTableSet(1): { + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + ckInfo(nil, []string{"colc", "colx"}, []string{"child_colc", "child_colx"}, sqlparser.SetNull), + }, + }, + parentFksWanted: map[TableSet][]vindexes.ParentFKInfo{}, + }, { + name: "Parent Foreign Keys Filtering", + analyzer: a, + allParentFks: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + pkInfo(nil, []string{"pcolc"}, []string{"colc"}), + pkInfo(nil, []string{"pcolb", "pcola"}, []string{"colb", "cola"}), + pkInfo(nil, []string{"pcolb"}, []string{"colb"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + pkInfo(nil, []string{"pcolb", "pcolx"}, []string{"colb", "colx"}), + }, + SingleTableSet(1): { + pkInfo(nil, []string{"pcolc", "pcolx"}, []string{"colc", "colx"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + pkInfo(nil, []string{"pcold", "pcolc"}, []string{"cold", "colc"}), + pkInfo(nil, []string{"pcold"}, []string{"cold"}), + pkInfo(nil, []string{"pcold", "pcolx"}, []string{"cold", "colx"}), + }, + }, + allChildFks: nil, + updExprs: updateExprs, + childFksWanted: map[TableSet][]vindexes.ChildFKInfo{}, + parentFksWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + }, + SingleTableSet(1): { + pkInfo(nil, []string{"pcolc", "pcolx"}, []string{"colc", "colx"}), + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + childFks, parentFks := tt.analyzer.filterForeignKeysUsingUpdateExpressions(tt.allChildFks, tt.allParentFks, tt.updExprs) + require.EqualValues(t, tt.childFksWanted, childFks) + require.EqualValues(t, tt.parentFksWanted, parentFks) + }) + } +} + +// TestGetInvolvedForeignKeys tests the functionality of getInvolvedForeignKeys. +func TestGetInvolvedForeignKeys(t *testing.T) { + cola := sqlparser.NewColName("cola") + colb := sqlparser.NewColName("colb") + colc := sqlparser.NewColName("colc") + cold := sqlparser.NewColName("cold") + tests := []struct { + name string + stmt sqlparser.Statement + analyzer *analyzer + childFksWanted map[TableSet][]vindexes.ChildFKInfo + parentFksWanted map[TableSet][]vindexes.ParentFKInfo + expectedErr string + }{ + { + name: "Delete Query", + stmt: &sqlparser.Delete{}, + analyzer: &analyzer{ + tables: &tableCollector{ + Tables: []TableInfo{ + tbl["t0"], + tbl["t1"], + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + "ks_unmanaged": vschemapb.Keyspace_FK_UNMANAGED, + }, + }, + }, + }, + childFksWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"col"}, []string{"col"}, sqlparser.Restrict), + ckInfo(nil, []string{"col1", "col2"}, []string{"ccol1", "ccol2"}, sqlparser.SetNull), + }, + }, + }, + { + name: "Update statement", + stmt: &sqlparser.Update{ + Exprs: sqlparser.UpdateExprs{ + &sqlparser.UpdateExpr{ + Name: cola, + Expr: sqlparser.NewIntLiteral("1"), + }, + &sqlparser.UpdateExpr{ + Name: colb, + Expr: &sqlparser.NullVal{}, + }, + &sqlparser.UpdateExpr{ + Name: colc, + Expr: sqlparser.NewIntLiteral("1"), + }, + &sqlparser.UpdateExpr{ + Name: cold, + Expr: &sqlparser.NullVal{}, + }, + }, + }, + analyzer: &analyzer{ + binder: &binder{ + direct: map[sqlparser.Expr]TableSet{ + cola: SingleTableSet(0), + colb: SingleTableSet(0), + colc: SingleTableSet(1), + cold: SingleTableSet(1), + }, + }, + tables: &tableCollector{ + Tables: []TableInfo{ + &RealTable{ + Table: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + ChildForeignKeys: []vindexes.ChildFKInfo{ + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + }, + ParentForeignKeys: []vindexes.ParentFKInfo{ + pkInfo(nil, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + pkInfo(nil, []string{"pcolc"}, []string{"colc"}), + pkInfo(nil, []string{"pcolb", "pcola"}, []string{"colb", "cola"}), + pkInfo(nil, []string{"pcolb"}, []string{"colb"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + pkInfo(nil, []string{"pcolb", "pcolx"}, []string{"colb", "colx"}), + }, + }, + }, + &RealTable{ + Table: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + ChildForeignKeys: []vindexes.ChildFKInfo{ + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + ckInfo(nil, []string{"colc", "colx"}, []string{"child_colc", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + }, + ParentForeignKeys: []vindexes.ParentFKInfo{ + pkInfo(nil, []string{"pcolc", "pcolx"}, []string{"colc", "colx"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + pkInfo(nil, []string{"pcold", "pcolc"}, []string{"cold", "colc"}), + pkInfo(nil, []string{"pcold"}, []string{"cold"}), + pkInfo(nil, []string{"pcold", "pcolx"}, []string{"cold", "colx"}), + }, + }, + }, + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + }, + }, + }, + }, + childFksWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + }, + SingleTableSet(1): { + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + ckInfo(nil, []string{"colc", "colx"}, []string{"child_colc", "child_colx"}, sqlparser.SetNull), + }, + }, + parentFksWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + }, + SingleTableSet(1): { + pkInfo(nil, []string{"pcolc", "pcolx"}, []string{"colc", "colx"}), + }, + }, + }, + { + name: "Replace Query", + stmt: &sqlparser.Insert{ + Action: sqlparser.ReplaceAct, + }, + analyzer: &analyzer{ + tables: &tableCollector{ + Tables: []TableInfo{ + tbl["t0"], + tbl["t1"], + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + "ks_unmanaged": vschemapb.Keyspace_FK_UNMANAGED, + }, + }, + }, + }, + childFksWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"col"}, []string{"col"}, sqlparser.Restrict), + ckInfo(nil, []string{"col1", "col2"}, []string{"ccol1", "ccol2"}, sqlparser.SetNull), + }, + }, + parentFksWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"colb"}, []string{"colb"}), + pkInfo(nil, []string{"colb1", "colb2"}, []string{"ccolb1", "ccolb2"}), + }, + }, + }, + { + name: "Insert Query", + stmt: &sqlparser.Insert{ + Action: sqlparser.InsertAct, + }, + analyzer: &analyzer{ + tables: &tableCollector{ + Tables: []TableInfo{ + tbl["t0"], + tbl["t1"], + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + "ks_unmanaged": vschemapb.Keyspace_FK_UNMANAGED, + }, + }, + }, + }, + childFksWanted: nil, + parentFksWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"colb"}, []string{"colb"}), + pkInfo(nil, []string{"colb1", "colb2"}, []string{"ccolb1", "ccolb2"}), + }, + }, + }, + { + name: "Insert Query with On Duplicate", + stmt: &sqlparser.Insert{ + Action: sqlparser.InsertAct, + OnDup: sqlparser.OnDup{ + &sqlparser.UpdateExpr{ + Name: cola, + Expr: sqlparser.NewIntLiteral("1"), + }, + &sqlparser.UpdateExpr{ + Name: colb, + Expr: &sqlparser.NullVal{}, + }, + }, + }, + analyzer: &analyzer{ + binder: &binder{ + direct: map[sqlparser.Expr]TableSet{ + cola: SingleTableSet(0), + colb: SingleTableSet(0), + }, + }, + tables: &tableCollector{ + Tables: []TableInfo{ + &RealTable{ + Table: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + ChildForeignKeys: []vindexes.ChildFKInfo{ + ckInfo(nil, []string{"col"}, []string{"col"}, sqlparser.Restrict), + ckInfo(nil, []string{"col1", "col2"}, []string{"ccol1", "ccol2"}, sqlparser.SetNull), + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + }, + ParentForeignKeys: []vindexes.ParentFKInfo{ + pkInfo(nil, []string{"colb"}, []string{"colb"}), + pkInfo(nil, []string{"colb1", "colb2"}, []string{"ccolb1", "ccolb2"}), + }, + }, + }, + tbl["t1"], + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + "ks_unmanaged": vschemapb.Keyspace_FK_UNMANAGED, + }, + }, + }, + }, + childFksWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + }, + }, + parentFksWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"colb"}, []string{"colb"}), + pkInfo(nil, []string{"colb1", "colb2"}, []string{"ccolb1", "ccolb2"}), + }, + }, + }, + { + name: "Insert error", + stmt: &sqlparser.Insert{}, + analyzer: &analyzer{ + tables: &tableCollector{ + Tables: []TableInfo{ + tbl["t2"], + tbl["t3"], + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + }, + }, + }, + }, + expectedErr: "undefined_ks keyspace not found", + }, + { + name: "Update error", + stmt: &sqlparser.Update{}, + analyzer: &analyzer{ + tables: &tableCollector{ + Tables: []TableInfo{ + tbl["t2"], + tbl["t3"], + }, + si: &FakeSI{ + KsForeignKeyMode: map[string]vschemapb.Keyspace_ForeignKeyMode{ + "ks": vschemapb.Keyspace_FK_MANAGED, + }, + }, + }, + }, + expectedErr: "undefined_ks keyspace not found", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + childFks, parentFks, err := tt.analyzer.getInvolvedForeignKeys(tt.stmt) + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) + return + } + require.EqualValues(t, tt.childFksWanted, childFks) + require.EqualValues(t, tt.parentFksWanted, parentFks) + }) + } +} + +func ckInfo(cTable *vindexes.Table, pCols []string, cCols []string, refAction sqlparser.ReferenceAction) vindexes.ChildFKInfo { + return vindexes.ChildFKInfo{ + Table: cTable, + ParentColumns: sqlparser.MakeColumns(pCols...), + ChildColumns: sqlparser.MakeColumns(cCols...), + OnDelete: refAction, + } +} + +func pkInfo(parentTable *vindexes.Table, pCols []string, cCols []string) vindexes.ParentFKInfo { + return vindexes.ParentFKInfo{ + Table: parentTable, + ParentColumns: sqlparser.MakeColumns(pCols...), + ChildColumns: sqlparser.MakeColumns(cCols...), + } +} diff --git a/go/vt/vtgate/semantics/info_schema.go b/go/vt/vtgate/semantics/info_schema.go index af050d5ff1b..f8c8f711901 100644 --- a/go/vt/vtgate/semantics/info_schema.go +++ b/go/vt/vtgate/semantics/info_schema.go @@ -23,6 +23,7 @@ import ( "vitess.io/vitess/go/vt/key" "vitess.io/vitess/go/vt/proto/query" topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vschemapb "vitess.io/vitess/go/vt/proto/vschema" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vtgate/vindexes" @@ -1712,3 +1713,7 @@ func (i *infoSchemaWithColumns) FindTableOrVindex(tbl sqlparser.TableName) (*vin func (i *infoSchemaWithColumns) ConnCollation() collations.ID { return i.inner.ConnCollation() } + +func (i *infoSchemaWithColumns) ForeignKeyMode(keyspace string) (vschemapb.Keyspace_ForeignKeyMode, error) { + return i.inner.ForeignKeyMode(keyspace) +} diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 0af935918f9..4e31d7ebd8e 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -23,6 +23,7 @@ import ( "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" topodatapb "vitess.io/vitess/go/vt/proto/topodata" + vschemapb "vitess.io/vitess/go/vt/proto/vschema" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" @@ -125,6 +126,11 @@ type ( // QuerySignature is used to identify shortcuts in the planning process QuerySignature QuerySignature + + // We store the child and parent foreign keys that are involved in the given query. + // The map is keyed by the tableset of the table that each of the foreign key belongs to. + childForeignKeysInvolved map[TableSet][]vindexes.ChildFKInfo + parentForeignKeysInvolved map[TableSet][]vindexes.ParentFKInfo } columnName struct { @@ -136,6 +142,8 @@ type ( 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) } ) @@ -153,6 +161,165 @@ func (st *SemTable) CopyDependencies(from, to sqlparser.Expr) { } } +// GetChildForeignKeysList gets the child foreign keys as a list. +func (st *SemTable) GetChildForeignKeysList() []vindexes.ChildFKInfo { + var childFkInfos []vindexes.ChildFKInfo + for _, infos := range st.childForeignKeysInvolved { + childFkInfos = append(childFkInfos, infos...) + } + return childFkInfos +} + +// GetParentForeignKeysList gets the parent foreign keys as a list. +func (st *SemTable) GetParentForeignKeysList() []vindexes.ParentFKInfo { + var parentFkInfos []vindexes.ParentFKInfo + for _, infos := range st.parentForeignKeysInvolved { + parentFkInfos = append(parentFkInfos, infos...) + } + return parentFkInfos +} + +// RemoveParentForeignKey removes the given foreign key from the parent foreign keys that sem table stores. +func (st *SemTable) RemoveParentForeignKey(fkToIgnore string) error { + for ts, fkInfos := range st.parentForeignKeysInvolved { + ti, err := st.TableInfoFor(ts) + if err != nil { + return err + } + vt := ti.GetVindexTable() + for idx, info := range fkInfos { + if info.String(vt) == fkToIgnore { + st.parentForeignKeysInvolved[ts] = append(fkInfos[0:idx], fkInfos[idx+1:]...) + return nil + } + } + } + return nil +} + +// RemoveNonRequiredForeignKeys prunes the list of foreign keys that the query involves. +// This function considers whether VTGate needs to validate all foreign keys +// or can delegate some of the responsibility to MySQL. +// In the latter case, the following types of foreign keys can be safely removed from our list: +// 1. Shard-scoped parent foreign keys: MySQL itself will reject a DML operation that violates these constraints. +// 2. Shard-scoped RESTRICT foreign keys: MySQL will also fail the operation if these foreign key constraints are breached. +func (st *SemTable) RemoveNonRequiredForeignKeys(verifyAllFks bool, getAction func(fk vindexes.ChildFKInfo) sqlparser.ReferenceAction) error { + if verifyAllFks { + return nil + } + // Go over all the parent foreign keys. + for ts, parentFKs := range st.parentForeignKeysInvolved { + ti, err := st.TableInfoFor(ts) + if err != nil { + return err + } + vt := ti.GetVindexTable() + var updatedParentFks []vindexes.ParentFKInfo + for _, fk := range parentFKs { + // Cross-keyspace foreign keys require verification. + if vt.Keyspace.Name != fk.Table.Keyspace.Name { + updatedParentFks = append(updatedParentFks, fk) + continue + } + // Non shard-scoped foreign keys require verification. + if !isShardScoped(fk.Table, vt, fk.ParentColumns, fk.ChildColumns) { + updatedParentFks = append(updatedParentFks, fk) + } + } + st.parentForeignKeysInvolved[ts] = updatedParentFks + } + + // Go over all the child foreign keys. + for ts, childFks := range st.childForeignKeysInvolved { + ti, err := st.TableInfoFor(ts) + if err != nil { + return err + } + vt := ti.GetVindexTable() + var updatedChildFks []vindexes.ChildFKInfo + for _, fk := range childFks { + // Cross-keyspace foreign keys require verification. + if vt.Keyspace.Name != fk.Table.Keyspace.Name { + updatedChildFks = append(updatedChildFks, fk) + continue + } + switch getAction(fk) { + case sqlparser.Cascade, sqlparser.SetNull, sqlparser.SetDefault: + updatedChildFks = append(updatedChildFks, fk) + continue + } + // sqlparser.Restrict, sqlparser.NoAction, sqlparser.DefaultAction + // all the actions means the same thing i.e. Restrict + // do not allow modification if there is a child row. + // Check if the restrict is shard scoped. + if !isShardScoped(vt, fk.Table, fk.ParentColumns, fk.ChildColumns) { + updatedChildFks = append(updatedChildFks, fk) + } + } + st.childForeignKeysInvolved[ts] = updatedChildFks + } + + return nil +} + +// isShardScoped checks if the foreign key constraint is shard-scoped or not. It uses the vindex information to make this call. +func isShardScoped(pTable *vindexes.Table, cTable *vindexes.Table, pCols sqlparser.Columns, cCols sqlparser.Columns) bool { + if !pTable.Keyspace.Sharded { + return true + } + + pPrimaryVdx := pTable.ColumnVindexes[0] + cPrimaryVdx := cTable.ColumnVindexes[0] + + // If the primary vindexes don't match between the parent and child table, + // we cannot infer that the fk constraint in shard scoped. + if cPrimaryVdx.Vindex != pPrimaryVdx.Vindex { + return false + } + + childFkContatined, childFkIndexes := cCols.Indexes(cPrimaryVdx.Columns) + if !childFkContatined { + // PrimaryVindex is not part of the foreign key constraint on the children side. + // So it is a cross-shard foreign key. + return false + } + + // We need to run the same check for the parent columns. + parentFkContatined, parentFkIndexes := pCols.Indexes(pPrimaryVdx.Columns) + if !parentFkContatined { + return false + } + + // Both the child and parent table contain the foreign key and that the vindexes are the same, + // now we need to make sure, that the indexes of both match. + // For example, consider the following tables, + // t1 (primary vindex (x,y)) + // t2 (primary vindex (a,b)) + // If we have a foreign key constraint from t1(x,y) to t2(b,a), then they are not shard scoped. + // Let's say in t1, (1,3) will be in -80 and (3,1) will be in 80-, then in t2 (1,3) will end up in 80-. + for i := range parentFkIndexes { + if parentFkIndexes[i] != childFkIndexes[i] { + return false + } + } + return true +} + +// ForeignKeysPresent returns whether there are any foreign key constraints left in the semantic table that require handling. +func (st *SemTable) ForeignKeysPresent() bool { + for _, fkInfos := range st.childForeignKeysInvolved { + if len(fkInfos) > 0 { + return true + } + } + for _, fkInfos := range st.parentForeignKeysInvolved { + if len(fkInfos) > 0 { + return true + } + } + return false +} + func (st *SemTable) SelectExprs(sel sqlparser.SelectStatement) sqlparser.SelectExprs { switch sel := sel.(type) { case *sqlparser.Select: diff --git a/go/vt/vtgate/semantics/semantic_state_test.go b/go/vt/vtgate/semantics/semantic_state_test.go index 98023bec6fc..ab855322d76 100644 --- a/go/vt/vtgate/semantics/semantic_state_test.go +++ b/go/vt/vtgate/semantics/semantic_state_test.go @@ -75,3 +75,676 @@ func fakeSchemaInfoTest() *FakeSI { } return si } + +// TestForeignKeysPresent tests the functionality of ForeignKeysPresent. +func TestForeignKeysPresent(t *testing.T) { + tests := []struct { + name string + st *SemTable + want bool + }{ + { + name: "Nil maps", + st: &SemTable{}, + want: false, + }, { + name: "Empty lists in the maps", + st: &SemTable{ + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(1): {}, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(1): {}, + }, + }, + want: false, + }, { + name: "Parent foriegn key exists", + st: &SemTable{ + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(1): {}, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(1): { + vindexes.ParentFKInfo{}, + }, + }, + }, + want: true, + }, { + name: "Child foriegn key exists", + st: &SemTable{ + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(1): { + vindexes.ChildFKInfo{}, + }, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(1): {}, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.st.ForeignKeysPresent()) + }) + } +} + +// TestIsShardScoped tests the functionality of isShardScoped. +func TestIsShardScoped(t *testing.T) { + hashVindex := &vindexes.Hash{} + xxhashVindex := &vindexes.XXHash{} + + tests := []struct { + name string + pTable *vindexes.Table + cTable *vindexes.Table + pCols sqlparser.Columns + cCols sqlparser.Columns + wantedShardScoped bool + }{ + { + name: "unsharded keyspace", + pTable: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "uks", Sharded: false}, + }, + wantedShardScoped: true, + }, + { + name: "Primary vindexes don't match", + pTable: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + }, + }, + }, + cTable: &vindexes.Table{ + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: xxhashVindex, + }, + }, + }, + wantedShardScoped: false, + }, + { + name: "Child primary vindex not part of the foreign key", + pTable: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + }, + }, + }, + cTable: &vindexes.Table{ + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("cola", "colb", "colc"), + }, + }, + }, + cCols: sqlparser.MakeColumns("colc", "colx", "cola"), + wantedShardScoped: false, + }, + { + name: "Parent primary vindex not part of the foreign key", + pTable: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("pcola", "pcolb", "pcolc"), + }, + }, + }, + cTable: &vindexes.Table{ + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("cola", "colb", "colc"), + }, + }, + }, + cCols: sqlparser.MakeColumns("colc", "colb", "cola"), + pCols: sqlparser.MakeColumns("pcolc", "pcolx", "pcola"), + wantedShardScoped: false, + }, + { + name: "Indexes order doesn't match", + pTable: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("pcola", "pcolb", "pcolc"), + }, + }, + }, + cTable: &vindexes.Table{ + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("cola", "colb", "colc"), + }, + }, + }, + cCols: sqlparser.MakeColumns("colc", "colb", "cola"), + pCols: sqlparser.MakeColumns("pcolc", "pcola", "pcolb"), + wantedShardScoped: false, + }, + { + name: "Is shard scoped", + pTable: &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("pcola", "pcolb", "pcolc"), + }, + }, + }, + cTable: &vindexes.Table{ + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("cola", "colb", "colc"), + }, + }, + }, + cCols: sqlparser.MakeColumns("colc", "colb", "cola"), + pCols: sqlparser.MakeColumns("pcolc", "pcolb", "pcola"), + wantedShardScoped: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.wantedShardScoped, isShardScoped(tt.pTable, tt.cTable, tt.pCols, tt.cCols)) + }) + } +} + +// TestGetChildForeignKeysList tests the function GetChildForeignKeysList +func TestGetChildForeignKeysList(t *testing.T) { + tests := []struct { + name string + semTable *SemTable + childFksWanted []vindexes.ChildFKInfo + }{ + { + name: "Collect all FKs", + semTable: &SemTable{ + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + }, + SingleTableSet(1): { + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + }, + }, + }, + childFksWanted: []vindexes.ChildFKInfo{ + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + }, + }, + { + name: "Nil Map", + semTable: &SemTable{ + childForeignKeysInvolved: nil, + }, + childFksWanted: nil, + }, + { + name: "Empty Map", + semTable: &SemTable{ + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): {}, + SingleTableSet(1): nil, + }, + }, + childFksWanted: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.ElementsMatch(t, tt.childFksWanted, tt.semTable.GetChildForeignKeysList()) + }) + } +} + +// TestGetParentForeignKeysList tests the function GetParentForeignKeysList +func TestGetParentForeignKeysList(t *testing.T) { + tests := []struct { + name string + semTable *SemTable + parentFksWanted []vindexes.ParentFKInfo + }{ + { + name: "Collect all FKs", + semTable: &SemTable{ + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"colb"}, []string{"child_colb"}), + pkInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}), + }, + SingleTableSet(1): { + pkInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}), + pkInfo(nil, []string{"cold"}, []string{"child_cold"}), + }, + }, + }, + parentFksWanted: []vindexes.ParentFKInfo{ + pkInfo(nil, []string{"colb"}, []string{"child_colb"}), + pkInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}), + pkInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}), + pkInfo(nil, []string{"cold"}, []string{"child_cold"}), + }, + }, + { + name: "Nil Map", + semTable: &SemTable{ + parentForeignKeysInvolved: nil, + }, + parentFksWanted: nil, + }, + { + name: "Empty Map", + semTable: &SemTable{ + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): {}, + SingleTableSet(1): nil, + }, + }, + parentFksWanted: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.ElementsMatch(t, tt.parentFksWanted, tt.semTable.GetParentForeignKeysList()) + }) + } +} + +// TestRemoveParentForeignKey tests the functionality of RemoveParentForeignKey +func TestRemoveParentForeignKey(t *testing.T) { + t1Table := &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + Name: sqlparser.NewIdentifierCS("t1"), + } + t2Table := &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + Name: sqlparser.NewIdentifierCS("t2"), + } + t3Table := &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks"}, + Name: sqlparser.NewIdentifierCS("t3"), + } + tests := []struct { + name string + semTable *SemTable + fkToIgnore string + parentFksWanted []vindexes.ParentFKInfo + expectedErr string + }{ + { + name: "Sucess", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, &RealTable{ + Table: t2Table, + }, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(t3Table, []string{"colb"}, []string{"child_colb"}), + pkInfo(t3Table, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}), + }, + SingleTableSet(1): { + pkInfo(t3Table, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}), + pkInfo(t3Table, []string{"cold"}, []string{"child_cold"}), + }, + }, + }, + fkToIgnore: "ks.t2child_coldks.t3cold", + parentFksWanted: []vindexes.ParentFKInfo{ + pkInfo(t3Table, []string{"colb"}, []string{"child_colb"}), + pkInfo(t3Table, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}), + pkInfo(t3Table, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}), + }, + }, { + name: "Foreign to ignore doesn't match any fk", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, &RealTable{ + Table: t2Table, + }, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(t3Table, []string{"colb"}, []string{"child_colb"}), + pkInfo(t3Table, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}), + }, + SingleTableSet(1): { + pkInfo(t3Table, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}), + pkInfo(t3Table, []string{"cold"}, []string{"child_cold"}), + }, + }, + }, + fkToIgnore: "incorrect name", + parentFksWanted: []vindexes.ParentFKInfo{ + pkInfo(t3Table, []string{"colb"}, []string{"child_colb"}), + pkInfo(t3Table, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}), + pkInfo(t3Table, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}), + pkInfo(t3Table, []string{"cold"}, []string{"child_cold"}), + }, + }, { + name: "Table information not found", + semTable: &SemTable{ + Tables: []TableInfo{}, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0).Merge(SingleTableSet(1)): { + pkInfo(t3Table, []string{"colb"}, []string{"child_colb"}), + pkInfo(t3Table, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}), + }, + }, + }, + expectedErr: "[BUG] should only be used for single tables", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.semTable.RemoveParentForeignKey(tt.fkToIgnore) + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) + return + } + require.ElementsMatch(t, tt.parentFksWanted, tt.semTable.GetParentForeignKeysList()) + }) + } +} + +// TestRemoveNonRequiredForeignKeys tests the functionality of RemoveNonRequiredForeignKeys. +func TestRemoveNonRequiredForeignKeys(t *testing.T) { + hashVindex := &vindexes.Hash{} + xxhashVindex := &vindexes.XXHash{} + t1Table := &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + Name: sqlparser.NewIdentifierCS("t1"), + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("cola"), + }, + }, + } + t2Table := &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + Name: sqlparser.NewIdentifierCS("t2"), + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: xxhashVindex, + Columns: sqlparser.MakeColumns("cola", "colb", "colc"), + }, + }, + } + t4Table := &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks", Sharded: true}, + Name: sqlparser.NewIdentifierCS("t4"), + ColumnVindexes: []*vindexes.ColumnVindex{ + { + Vindex: hashVindex, + Columns: sqlparser.MakeColumns("cola"), + }, + }, + } + t3Table := &vindexes.Table{ + Keyspace: &vindexes.Keyspace{Name: "ks2"}, + Name: sqlparser.NewIdentifierCS("t3"), + } + tests := []struct { + name string + verifyAllFks bool + semTable *SemTable + expectedErr string + childFkWanted map[TableSet][]vindexes.ChildFKInfo + parentFkWanted map[TableSet][]vindexes.ParentFKInfo + }{ + { + name: "VerifyAllFks specified", + verifyAllFks: true, + semTable: &SemTable{ + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + }, + SingleTableSet(1): { + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + ckInfo(nil, []string{"colc", "colx"}, []string{"child_colc", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + }, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + pkInfo(nil, []string{"pcolc"}, []string{"colc"}), + pkInfo(nil, []string{"pcolb", "pcola"}, []string{"colb", "cola"}), + pkInfo(nil, []string{"pcolb"}, []string{"colb"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + pkInfo(nil, []string{"pcolb", "pcolx"}, []string{"colb", "colx"}), + }, + }, + }, + childFkWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(nil, []string{"colb"}, []string{"child_colb"}, sqlparser.Restrict), + ckInfo(nil, []string{"cola", "colx"}, []string{"child_cola", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + }, + SingleTableSet(1): { + ckInfo(nil, []string{"cold"}, []string{"child_cold"}, sqlparser.Restrict), + ckInfo(nil, []string{"colc", "colx"}, []string{"child_colc", "child_colx"}, sqlparser.SetNull), + ckInfo(nil, []string{"colx", "coly"}, []string{"child_colx", "child_coly"}, sqlparser.Cascade), + }, + }, + parentFkWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(nil, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + pkInfo(nil, []string{"pcolc"}, []string{"colc"}), + pkInfo(nil, []string{"pcolb", "pcola"}, []string{"colb", "cola"}), + pkInfo(nil, []string{"pcolb"}, []string{"colb"}), + pkInfo(nil, []string{"pcola"}, []string{"cola"}), + pkInfo(nil, []string{"pcolb", "pcolx"}, []string{"colb", "colx"}), + }, + }, + }, + { + name: "Filtering - Keep cross keyspace parent foreign key", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, + &RealTable{ + Table: t2Table, + }, + }, + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{}, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(t3Table, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + }, + SingleTableSet(1): { + pkInfo(t3Table, []string{"pcolc", "pcolx"}, []string{"colc", "colx"}), + }, + }, + }, + childFkWanted: map[TableSet][]vindexes.ChildFKInfo{}, + parentFkWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(t3Table, []string{"pcola", "pcolx"}, []string{"cola", "colx"}), + }, + SingleTableSet(1): { + pkInfo(t3Table, []string{"pcolc", "pcolx"}, []string{"colc", "colx"}), + }, + }, + }, + { + name: "Filtering - Shard scoped parent foreign keys", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, + &RealTable{ + Table: t2Table, + }, + }, + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{}, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): { + pkInfo(t4Table, []string{"cola", "colx"}, []string{"cola", "colx"}), + }, + SingleTableSet(1): { + pkInfo(t4Table, []string{"colc", "colx"}, []string{"colc", "colx"}), + }, + }, + }, + childFkWanted: map[TableSet][]vindexes.ChildFKInfo{}, + parentFkWanted: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0): nil, + SingleTableSet(1): { + pkInfo(t4Table, []string{"colc", "colx"}, []string{"colc", "colx"}), + }, + }, + }, + { + name: "Filtering - Keep cross keyspace child foreign key", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, + &RealTable{ + Table: t2Table, + }, + }, + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(t3Table, []string{"cola", "colx"}, []string{"cola", "colx"}, sqlparser.Restrict), + }, + SingleTableSet(1): { + ckInfo(t3Table, []string{"colc", "colx"}, []string{"colc", "colx"}, sqlparser.Cascade), + }, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{}, + }, + childFkWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(t3Table, []string{"cola", "colx"}, []string{"cola", "colx"}, sqlparser.Restrict), + }, + SingleTableSet(1): { + ckInfo(t3Table, []string{"colc", "colx"}, []string{"colc", "colx"}, sqlparser.Cascade), + }, + }, + parentFkWanted: map[TableSet][]vindexes.ParentFKInfo{}, + }, + { + name: "Filtering - Remove Restrict shard scoped foreign keys", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, + &RealTable{ + Table: t2Table, + }, + }, + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(t4Table, []string{"cola", "colx"}, []string{"cola", "colx"}, sqlparser.Restrict), + ckInfo(t4Table, []string{"cola", "coly"}, []string{"cola", "coly"}, sqlparser.Cascade), + }, + SingleTableSet(1): { + ckInfo(t4Table, []string{"colc", "colx"}, []string{"colc", "colx"}, sqlparser.Restrict), + }, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{}, + }, + childFkWanted: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0): { + ckInfo(t4Table, []string{"cola", "coly"}, []string{"cola", "coly"}, sqlparser.Cascade), + }, + SingleTableSet(1): { + ckInfo(t4Table, []string{"colc", "colx"}, []string{"colc", "colx"}, sqlparser.Restrict), + }, + }, + parentFkWanted: map[TableSet][]vindexes.ParentFKInfo{}, + }, + { + name: "Error - Reading table info for parent foreign keys", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, + &RealTable{ + Table: t2Table, + }, + }, + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{}, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{ + SingleTableSet(0).Merge(SingleTableSet(1)): {}, + }, + }, + expectedErr: "[BUG] should only be used for single tables", + }, + { + name: "Error - Reading table info for child foreign keys", + semTable: &SemTable{ + Tables: []TableInfo{ + &RealTable{ + Table: t1Table, + }, + &RealTable{ + Table: t2Table, + }, + }, + childForeignKeysInvolved: map[TableSet][]vindexes.ChildFKInfo{ + SingleTableSet(0).Merge(SingleTableSet(1)): {}, + }, + parentForeignKeysInvolved: map[TableSet][]vindexes.ParentFKInfo{}, + }, + expectedErr: "[BUG] should only be used for single tables", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.semTable.RemoveNonRequiredForeignKeys(tt.verifyAllFks, vindexes.DeleteAction) + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) + return + } + require.EqualValues(t, tt.childFkWanted, tt.semTable.childForeignKeysInvolved) + require.EqualValues(t, tt.parentFkWanted, tt.semTable.parentForeignKeysInvolved) + }) + } +} diff --git a/go/vt/vtgate/vindexes/foreign_keys.go b/go/vt/vtgate/vindexes/foreign_keys.go index 3fcbc719624..db984462b25 100644 --- a/go/vt/vtgate/vindexes/foreign_keys.go +++ b/go/vt/vtgate/vindexes/foreign_keys.go @@ -114,115 +114,9 @@ func NewChildFkInfo(childTbl *Table, fkDef *sqlparser.ForeignKeyDefinition) Chil } } -// ParentFKsNeedsHandling returns all the parent fk constraints on this table that are not shard scoped. -func (t *Table) ParentFKsNeedsHandling(verifyAllFKs bool, fkToIgnore string) (fks []ParentFKInfo) { - for _, fk := range t.ParentForeignKeys { - // Check if we need to specifically ignore this foreign key - if fkToIgnore != "" && fk.String(t) == fkToIgnore { - continue - } - - // If we require all the foreign keys, add them all. - if verifyAllFKs { - fks = append(fks, fk) - continue - } - - // If the keyspaces are different, then the fk definition - // is going to go across shards. - if fk.Table.Keyspace.Name != t.Keyspace.Name { - fks = append(fks, fk) - continue - } - // If the keyspaces match and they are unsharded, then the fk defintion - // is shard-scoped. - if !t.Keyspace.Sharded { - continue - } - - if !isShardScoped(fk.Table, t, fk.ParentColumns, fk.ChildColumns) { - fks = append(fks, fk) - } - } - return -} - -// 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(verifyAllFKs bool, getAction func(fk ChildFKInfo) sqlparser.ReferenceAction) (fks []ChildFKInfo) { - // If we require all the foreign keys, return the entire list. - if verifyAllFKs { - return t.ChildForeignKeys - } - for _, fk := range t.ChildForeignKeys { - // If the keyspaces are different, then the fk definition - // is going to go across shards. - if fk.Table.Keyspace.Name != t.Keyspace.Name { - fks = append(fks, fk) - continue - } - // If the action is not Restrict, then it needs a cascade. - switch getAction(fk) { - case sqlparser.Cascade, sqlparser.SetNull, sqlparser.SetDefault: - fks = append(fks, fk) - continue - } - // sqlparser.Restrict, sqlparser.NoAction, sqlparser.DefaultAction - // all the actions means the same thing i.e. Restrict - // do not allow modification if there is a child row. - // Check if the restrict is shard scoped. - if !isShardScoped(t, fk.Table, fk.ParentColumns, fk.ChildColumns) { - fks = append(fks, fk) - } - } - 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 - } - - pPrimaryVdx := pTable.ColumnVindexes[0] - cPrimaryVdx := cTable.ColumnVindexes[0] - - // If the primary vindexes don't match between the parent and child table, - // we cannot infer that the fk constraint in shard scoped. - if cPrimaryVdx.Vindex != pPrimaryVdx.Vindex { - return false - } - - childFkContatined, childFkIndexes := cCols.Indexes(cPrimaryVdx.Columns) - if !childFkContatined { - // PrimaryVindex is not part of the foreign key constraint on the children side. - // So it is a cross-shard foreign key. - return false - } - - // We need to run the same check for the parent columns. - parentFkContatined, parentFkIndexes := pCols.Indexes(pPrimaryVdx.Columns) - if !parentFkContatined { - return false - } - - // Both the child and parent table contain the foreign key and that the vindexes are the same, - // now we need to make sure, that the indexes of both match. - // For example, consider the following tables, - // t1 (primary vindex (x,y)) - // t2 (primary vindex (a,b)) - // If we have a foreign key constraint from t1(x,y) to t2(b,a), then they are not shard scoped. - // Let's say in t1, (1,3) will be in -80 and (3,1) will be in 80-, then in t2 (1,3) will end up in 80-. - for i := range parentFkIndexes { - if parentFkIndexes[i] != childFkIndexes[i] { - return false - } - } - return true -} - // AddForeignKey is for testing only. func (vschema *VSchema) AddForeignKey(ksname, childTableName string, fkConstraint *sqlparser.ForeignKeyDefinition) error { ks, ok := vschema.Keyspaces[ksname] diff --git a/go/vt/vtgate/vindexes/foreign_keys_test.go b/go/vt/vtgate/vindexes/foreign_keys_test.go deleted file mode 100644 index 147614edcbf..00000000000 --- a/go/vt/vtgate/vindexes/foreign_keys_test.go +++ /dev/null @@ -1,314 +0,0 @@ -/* -Copyright 2023 The Vitess Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package vindexes - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "vitess.io/vitess/go/vt/sqlparser" -) - -var ( - uks = &Keyspace{Name: "uks"} - uks2 = &Keyspace{Name: "uks2"} - sks = &Keyspace{Name: "sks", Sharded: true} -) - -// TestTable_CrossShardParentFKs tests the functionality of the method CrossShardParentFKs. -func TestTable_CrossShardParentFKs(t *testing.T) { - col1Vindex := &ColumnVindex{ - Name: "v1", - Vindex: binVindex, - Columns: sqlparser.MakeColumns("col1"), - } - col4DiffVindex := &ColumnVindex{ - Name: "v2", - Vindex: binOnlyVindex, - Columns: sqlparser.MakeColumns("col4"), - } - col123Vindex := &ColumnVindex{ - Name: "v2", - Vindex: binVindex, - Columns: sqlparser.MakeColumns("col1", "col2", "col3"), - } - col456Vindex := &ColumnVindex{ - Name: "v2", - Vindex: binVindex, - Columns: sqlparser.MakeColumns("col4", "col5", "col6"), - } - - unshardedTbl := &Table{ - Name: sqlparser.NewIdentifierCS("t1"), - Keyspace: uks2, - } - shardedSingleColTblWithDiffVindex := &Table{ - Name: sqlparser.NewIdentifierCS("t1"), - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col4DiffVindex}, - } - shardedMultiColTbl := &Table{ - Name: sqlparser.NewIdentifierCS("t1"), - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col456Vindex}, - } - - tests := []struct { - name string - table *Table - wantCrossShardFKTables []string - verifyAllFKs bool - fkToIgnore string - }{{ - name: "No Parent FKs", - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: sks, - }, - wantCrossShardFKTables: []string{}, - }, { - name: "Unsharded keyspace", - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: uks2, - ParentForeignKeys: []ParentFKInfo{pkInfo(unshardedTbl, []string{"col4"}, []string{"col1"})}, - }, - wantCrossShardFKTables: []string{}, - }, { - name: "Unsharded keyspace with verify all FKs", - verifyAllFKs: true, - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: uks2, - ParentForeignKeys: []ParentFKInfo{pkInfo(unshardedTbl, []string{"col4"}, []string{"col1"})}, - }, - wantCrossShardFKTables: []string{"t1"}, - }, { - name: "Keyspaces don't match", // parent table is on uks2 - table: &Table{ - Keyspace: uks, - ParentForeignKeys: []ParentFKInfo{pkInfo(unshardedTbl, []string{"col4"}, []string{"col1"})}, - }, - wantCrossShardFKTables: []string{"t1"}, - }, { - name: "Keyspaces don't match with ignore fk", // parent table is on uks2 - fkToIgnore: "uks.col1uks2.t1col4", - table: &Table{ - Keyspace: uks, - ParentForeignKeys: []ParentFKInfo{pkInfo(unshardedTbl, []string{"col4"}, []string{"col1"})}, - }, - wantCrossShardFKTables: []string{}, - }, { - name: "Unsharded keyspace with verify all FKs and fk to ignore", - verifyAllFKs: true, - fkToIgnore: "uks2.col1uks2.t1col4", - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: uks2, - ParentForeignKeys: []ParentFKInfo{pkInfo(unshardedTbl, []string{"col4"}, []string{"col1"})}, - }, - wantCrossShardFKTables: []string{}, - }, { - name: "Column Vindexes don't match", // primary vindexes on different vindex type - table: &Table{ - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col1Vindex}, - ParentForeignKeys: []ParentFKInfo{pkInfo(shardedSingleColTblWithDiffVindex, []string{"col4"}, []string{"col1"})}, - }, - wantCrossShardFKTables: []string{"t1"}, - }, { - name: "child table foreign key does not contain primary vindex columns", - table: &Table{ - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col123Vindex}, - ParentForeignKeys: []ParentFKInfo{pkInfo(shardedMultiColTbl, []string{"col4", "col5", "col6"}, []string{"col3", "col9", "col1"})}, - }, - wantCrossShardFKTables: []string{"t1"}, - }, { - name: "Parent FK doesn't contain primary vindex", - table: &Table{ - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col123Vindex}, - ParentForeignKeys: []ParentFKInfo{pkInfo(shardedMultiColTbl, []string{"col4", "col9", "col6"}, []string{"col1", "col2", "col3"})}, - }, - wantCrossShardFKTables: []string{"t1"}, - }, { - name: "Indexes of the two FKs with column vindexes don't line up", - table: &Table{ - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col123Vindex}, - ParentForeignKeys: []ParentFKInfo{pkInfo(shardedMultiColTbl, []string{"col4", "col9", "col5", "col6"}, []string{"col1", "col2", "col3", "col9"})}, - }, - wantCrossShardFKTables: []string{"t1"}, - }, { - name: "Shard scoped foreign key constraint", - table: &Table{ - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col123Vindex}, - ParentForeignKeys: []ParentFKInfo{pkInfo(shardedMultiColTbl, []string{"col4", "col9", "col5", "col6", "colc"}, []string{"col1", "cola", "col2", "col3", "colb"})}, - }, - wantCrossShardFKTables: []string{}, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - crossShardFks := tt.table.ParentFKsNeedsHandling(tt.verifyAllFKs, tt.fkToIgnore) - var crossShardFkTables []string - for _, fk := range crossShardFks { - crossShardFkTables = append(crossShardFkTables, fk.Table.Name.String()) - } - require.ElementsMatch(t, tt.wantCrossShardFKTables, crossShardFkTables) - }) - } -} - -func pkInfo(parentTable *Table, pCols []string, cCols []string) ParentFKInfo { - return ParentFKInfo{ - Table: parentTable, - ParentColumns: sqlparser.MakeColumns(pCols...), - ChildColumns: sqlparser.MakeColumns(cCols...), - } -} - -// TestChildFKs tests the ChildFKsNeedsHandling method is provides the child foreign key table whose -// rows needs to be managed by vitess. -func TestChildFKs(t *testing.T) { - col1Vindex := &ColumnVindex{ - Name: "v1", - Vindex: binVindex, - Columns: sqlparser.MakeColumns("col1"), - } - col4DiffVindex := &ColumnVindex{ - Name: "v2", - Vindex: binOnlyVindex, - Columns: sqlparser.MakeColumns("col4"), - } - - unshardedTbl := &Table{ - Name: sqlparser.NewIdentifierCS("t1"), - Keyspace: uks2, - } - shardedSingleColTbl := &Table{ - Name: sqlparser.NewIdentifierCS("t1"), - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col1Vindex}, - } - shardedSingleColTblWithDiffVindex := &Table{ - Name: sqlparser.NewIdentifierCS("t1"), - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col4DiffVindex}, - } - - tests := []struct { - verifyAllFKs bool - name string - table *Table - expChildTbls []string - }{{ - name: "No Parent FKs", - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: sks, - }, - expChildTbls: []string{}, - }, { - name: "restrict unsharded", - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: uks2, - ChildForeignKeys: []ChildFKInfo{ckInfo(unshardedTbl, []string{"col4"}, []string{"col1"}, sqlparser.Restrict)}, - }, - expChildTbls: []string{}, - }, { - name: "restrict unsharded with verify all fks", - verifyAllFKs: true, - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: uks2, - ChildForeignKeys: []ChildFKInfo{ckInfo(unshardedTbl, []string{"col4"}, []string{"col1"}, sqlparser.Restrict)}, - }, - expChildTbls: []string{"t1"}, - }, { - name: "restrict shard scoped", - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: sks, - ChildForeignKeys: []ChildFKInfo{ckInfo(shardedSingleColTbl, []string{"col1"}, []string{"col1"}, sqlparser.Restrict)}, - }, - expChildTbls: []string{}, - }, { - name: "restrict shard scoped with verify all fks", - verifyAllFKs: true, - table: &Table{ - ColumnVindexes: []*ColumnVindex{col1Vindex}, - Keyspace: sks, - ChildForeignKeys: []ChildFKInfo{ckInfo(shardedSingleColTbl, []string{"col1"}, []string{"col1"}, sqlparser.Restrict)}, - }, - expChildTbls: []string{"t1"}, - }, { - name: "restrict Keyspaces don't match", - table: &Table{ - Keyspace: uks, - ChildForeignKeys: []ChildFKInfo{ckInfo(shardedSingleColTbl, []string{"col1"}, []string{"col1"}, sqlparser.Restrict)}, - }, - expChildTbls: []string{"t1"}, - }, { - name: "restrict cross shard", - table: &Table{ - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col1Vindex}, - ChildForeignKeys: []ChildFKInfo{ckInfo(shardedSingleColTblWithDiffVindex, []string{"col4"}, []string{"col1"}, sqlparser.Restrict)}, - }, - expChildTbls: []string{"t1"}, - }, { - name: "cascade unsharded", - table: &Table{ - Keyspace: uks2, - ColumnVindexes: []*ColumnVindex{col1Vindex}, - ChildForeignKeys: []ChildFKInfo{ckInfo(unshardedTbl, []string{"col4"}, []string{"col1"}, sqlparser.Cascade)}, - }, - expChildTbls: []string{"t1"}, - }, { - name: "cascade cross shard", - table: &Table{ - Keyspace: sks, - ColumnVindexes: []*ColumnVindex{col1Vindex}, - ChildForeignKeys: []ChildFKInfo{ckInfo(shardedSingleColTblWithDiffVindex, []string{"col4"}, []string{"col1"}, sqlparser.Cascade)}, - }, - 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(tt.verifyAllFKs, deleteAction) - var actualChildTbls []string - for _, fk := range childFks { - actualChildTbls = append(actualChildTbls, fk.Table.Name.String()) - } - require.ElementsMatch(t, tt.expChildTbls, actualChildTbls) - }) - } -} - -func ckInfo(cTable *Table, pCols []string, cCols []string, refAction sqlparser.ReferenceAction) ChildFKInfo { - return ChildFKInfo{ - Table: cTable, - ParentColumns: sqlparser.MakeColumns(pCols...), - ChildColumns: sqlparser.MakeColumns(cCols...), - OnDelete: refAction, - } -}