Refactor: foreign key in semantic analysis phase (#14273)

Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: Manan Gupta <[email protected]>
vitessio · Oct 16, 2023 · a44d799 · a44d799
1 parent 0f751fb
commit a44d799
Show file tree

Hide file tree

Showing 18 changed files with 1,647 additions and 855 deletions.
diff --git 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
@@ -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
@@ -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
diff --git 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) {