cleanup

Author: angelamayxie

enginetest/queries/update_queries.go

@@ -482,8 +482,6 @@ var UpdateScriptTests = []ScriptTest{
 		},
 		Assertions: []ScriptTestAssertion{
 			{
-				// TODO: Foreign key constraints are not honored for UDPATE ... JOIN statements
-				Skip:        true,
 				Query:       "UPDATE orders o JOIN customers c ON o.customer_id = c.id SET o.customer_id = 123 where o.customer_id != 1;",
 				ExpectedErr: sql.ErrForeignKeyChildViolation,
 			},
@@ -510,16 +508,12 @@ var UpdateScriptTests = []ScriptTest{
 		},
 		Assertions: []ScriptTestAssertion{
 			{
-				// TODO: Foreign key constraints are not honored for UDPATE ... JOIN statements
-				Skip: true,
 				Query: `UPDATE child1 c1
 						JOIN child2 c2 ON c1.id = 10 AND c2.id = 20
 						SET c1.p1_id = 999, c2.p2_id = 3;`,
 				ExpectedErr: sql.ErrForeignKeyChildViolation,
 			},
 			{
-				// TODO: Foreign key constraints are not honored for UDPATE ... JOIN statements
-				Skip: true,
 				Query: `UPDATE child1 c1
 						JOIN child2 c2 ON c1.id = 10 AND c2.id = 20
 						SET c1.p1_id = 3, c2.p2_id = 999;`,

sql/analyzer/apply_foreign_keys.go

@@ -138,50 +138,29 @@ func applyForeignKeysToNodes(ctx *sql.Context, a *Analyzer, n sql.Node, cache *f
 				if err != nil {
 					return nil, transform.SameTree, err
 				}
-
-				fkTbl, ok := updateDest.(sql.ForeignKeyTable)
-				if !ok {
-					continue
-				}
-				fkEditor, err := getForeignKeyEditor(ctx, a, fkTbl, cache, fkChain, false)
+				fkHandler, err :=
+					getForeignKeyHandlerFromUpdateDestination(updateDest, ctx, a, cache, fkChain, updateTarget)
 				if err != nil {
 					return nil, transform.SameTree, err
 				}
-				if fkEditor == nil {
-					continue
-				}
-				fkHandlerMap[tableName] = &plan.ForeignKeyHandler{
-					Table:        fkTbl,
-					Sch:          updateDest.Schema(),
-					OriginalNode: updateTarget,
-					Editor:       fkEditor,
-					AllUpdaters:  fkChain.GetUpdaters(),
+				if fkHandler == nil {
+					fkHandlerMap[tableName] = updateTarget
+				} else {
+					fkHandlerMap[tableName] = fkHandler
 				}
 			}
 			uj := plan.NewUpdateJoin(fkHandlerMap, n.Child.(*plan.UpdateJoin).Child)
 			nn, err := n.WithChildren(uj)
 			return nn, transform.NewTree, err
 		default:
-			fkTbl, ok := updateDest.(sql.ForeignKeyTable)
-			// If foreign keys aren't supported then we return
-			if !ok {
-				return n, transform.SameTree, nil
-			}
-
-			fkEditor, err := getForeignKeyEditor(ctx, a, fkTbl, cache, fkChain, false)
+			fkHandler, err := getForeignKeyHandlerFromUpdateDestination(updateDest, ctx, a, cache, fkChain, n.Child)
 			if err != nil {
 				return nil, transform.SameTree, err
 			}
-			if fkEditor == nil {
+			if fkHandler == nil {
 				return n, transform.SameTree, nil
 			}
-			nn, err := n.WithChildren(&plan.ForeignKeyHandler{
-				Table:        fkTbl,
-				Sch:          updateDest.Schema(),
-				OriginalNode: n.Child,
-				Editor:       fkEditor,
-				AllUpdaters:  fkChain.GetUpdaters(),
-			})
+			nn, err := n.WithChildren(fkHandler)
 			return nn, transform.NewTree, err
 		}
 	case *plan.DeleteFrom:
@@ -480,6 +459,30 @@ func getForeignKeyRefActions(ctx *sql.Context, a *Analyzer, tbl sql.ForeignKeyTa
 	return fkEditor, nil
 }
 
+func getForeignKeyHandlerFromUpdateDestination(updateDest sql.UpdatableTable, ctx *sql.Context, a *Analyzer,
+	cache *foreignKeyCache, fkChain foreignKeyChain, originalNode sql.Node) (*plan.ForeignKeyHandler, error) {
+	fkTbl, ok := updateDest.(sql.ForeignKeyTable)
+	if !ok {
+		return nil, nil
+	}
+
+	fkEditor, err := getForeignKeyEditor(ctx, a, fkTbl, cache, fkChain, false)
+	if err != nil {
+		return nil, err
+	}
+	if fkEditor == nil {
+		return nil, nil
+	}
+
+	return &plan.ForeignKeyHandler{
+		Table:        fkTbl,
+		Sch:          updateDest.Schema(),
+		OriginalNode: originalNode,
+		Editor:       fkEditor,
+		AllUpdaters:  fkChain.GetUpdaters(),
+	}, nil
+}
+
 // resolveSchemaDefaults resolves the default values for the schema of |table|. This is primarily needed for column
 // default value expressions, since those don't get resolved during the planbuilder phase and assignExecIndexes
 // doesn't traverse through the ForeignKeyEditors and referential actions to find all of them. In addition to resolving

sql/analyzer/assign_update_join.go

@@ -34,7 +34,7 @@ func modifyUpdateExprsForJoin(ctx *sql.Context, a *Analyzer, n sql.Node, scope *
 			return n, transform.SameTree, nil
 		}
 
-		updateTargets, err := updateTargetsByTable(us, jn)
+		updateTargets, err := targetsByTable(us, jn)
 		if err != nil {
 			return nil, transform.SameTree, err
 		}
@@ -51,8 +51,8 @@ func modifyUpdateExprsForJoin(ctx *sql.Context, a *Analyzer, n sql.Node, scope *
 	return n, transform.SameTree, nil
 }
 
-// rowUpdatersByTable maps a set of tables to their RowUpdater objects.
-func updateTargetsByTable(node sql.Node, ij sql.Node) (map[string]sql.Node, error) {
+// targetsByTable maps a set of table names to their corresponding Node
+func targetsByTable(node sql.Node, ij sql.Node) (map[string]sql.Node, error) {
 	namesOfTableToBeUpdated := getTablesToBeUpdated(node)
 	resolvedTables := getTablesByName(ij)
 

sql/plan/update_join.go

@@ -25,7 +25,7 @@ type UpdateJoin struct {
 	UnaryNode
 }
 
-// NewUpdateJoin returns an *UpdateJoin node.
+// NewUpdateJoin returns a new *UpdateJoin node.
 func NewUpdateJoin(updateTargets map[string]sql.Node, child sql.Node) *UpdateJoin {
 	return &UpdateJoin{
 		UpdateTargets: updateTargets,
@@ -54,11 +54,6 @@ func (u *UpdateJoin) DebugString() string {
 
 // GetUpdatable returns an updateJoinTable which implements sql.UpdatableTable.
 func (u *UpdateJoin) GetUpdatable() sql.UpdatableTable {
-	// TODO: UpdateJoin can update multiple tables, but this interface only allows for a single table.
-	//       Additionally, updatableJoinTable doesn't implement interfaces that other parts of the code
-	//       expect, so UpdateJoins don't always work correctly. For example, because updatableJoinTable
-	//       doesn't implement ForeignKeyTable, UpdateJoin statements don't enforce foreign key checks.
-	//       We should revamp this function so that we can communicate multiple tables being updated.
 	return &UpdatableJoinTable{
 		UpdateTargets: u.UpdateTargets,
 		joinNode:      u.Child.(*UpdateSource).Child,
@@ -74,10 +69,6 @@ func (u *UpdateJoin) WithChildren(children ...sql.Node) (sql.Node, error) {
 	return NewUpdateJoin(u.UpdateTargets, children[0]), nil
 }
 
-func (u *UpdateJoin) WithUpdateTargets(updateTargets map[string]sql.Node) *UpdateJoin {
-	return NewUpdateJoin(updateTargets, u.Child)
-}
-
 func (u *UpdateJoin) IsReadOnly() bool {
 	return false
 }
@@ -103,7 +94,7 @@ func getUpdaters(updateTargets map[string]sql.Node, ctx *sql.Context) (map[strin
 	return updaterMap, nil
 }
 
-// updatableJoinTable manages the update of multiple tables.
+// UpdatableJoinTable manages the update of multiple tables.
 type UpdatableJoinTable struct {
 	UpdateTargets map[string]sql.Node
 	joinNode      sql.Node