fix foreign-key cascade review findings

executeUpdateFrom implemented a primary-key move as DELETE + INSERT, so with
a cascading foreign key the DELETE fired ON DELETE CASCADE and dropped every
child (or SET NULL/SET DEFAULT rewrote it) while ON UPDATE actions never ran.
Apply the whole post-update image through Session.UpdateByPK so the session
stages the key move, detects 23505 collisions, and fires ON UPDATE actions.

Document that cascading actions are applied with the statement in autocommit
and no later than COMMIT inside an explicit transaction, that a NO
ACTION/RESTRICT violation in a transaction is detected at COMMIT while a
cascade failure rolls back to the statement savepoint, that a mutual cycle can
only exist in a hand-authored catalog, and that cascading point DML costs
O(FK component) rather than O(1).

Hold the database-wide foreign-key write gate across the cascade
classification and Begin for participating tables, so a concurrent CREATE
TABLE ... REFERENCES cannot change the decision before the matching staged
transaction exists. The gate is released before Commit, which reacquires it.

Adds regression tests for UPDATE ... FROM key moves with ON UPDATE CASCADE
(with and without ON DELETE CASCADE) and for statement-level versus
COMMIT-level error timing inside an explicit transaction.

pkg/capabilities/capabilities_test.go

@@ -165,8 +165,9 @@ func TestForeignKeyIsAdvertised(t *testing.T) {
 	}
 	for _, want := range []string{
 		"UNIQUE", "COMMIT", "MATCH SIMPLE", "MATCH keyword", "self",
-		"CASCADE", "SET NULL", "SET DEFAULT", "NOT NULL", "immediately", "transitively",
-		"DEFERRABLE", "0A000",
+		"CASCADE", "SET NULL", "SET DEFAULT", "NOT NULL", "transitively",
+		"autocommit", "no later than COMMIT", "statement savepoint", "hand-authored catalog",
+		"O(FK component)", "DEFERRABLE", "0A000",
 		"PRAGMA foreign_keys", "rejects OFF", "foreign_key_list", "database-wide foreign-key write gate",
 		"DROP/RENAME COLUMN", "DROP TABLE", "pg_constraint",
 	} {

pkg/executor/executor.go

@@ -3494,37 +3494,25 @@ func (e *Executor) executeUpdateFrom(stmt *parser.UpdateStmt, schema *storage.Sc
 			if newPKErr != nil {
 				return newPKErr
 			}
-			if newPK != plan.oldPK {
-				if _, gerr := e.session.GetByPK(tableName, newPK); gerr == nil {
-					return &storage.PrimaryKeyViolationError{Table: tableName, Key: newPK}
-				} else if gerr != storage.ErrKeyNotFound {
-					return gerr
-				}
-				if _, deleted, derr := e.session.DeleteByPK(tableName, plan.oldPK); derr != nil {
-					return derr
-				} else if !deleted {
-					continue
-				}
-				if _, ierr := e.session.InsertWithRowID(tableName, plan.row); ierr != nil {
-					return ierr
-				}
-			} else {
-				updates := make(storage.Row, len(schema.Columns))
-				for _, col := range schema.Columns {
-					if v, ok := lookupRowValue(plan.row, col.Name); ok {
-						updates[col.Name] = v
-					}
-				}
-				_, updated, uerr := e.session.UpdateByPK(tableName, newPK, func(storage.Row) (storage.Row, error) {
-					return updates, nil
-				})
-				if uerr != nil {
-					return uerr
-				}
-				if !updated {
-					continue
+			// Apply the whole post-update image through one update so a primary-key
+			// move is a real UPDATE: the session stages the key move (with the
+			// duplicate check) and fires ON UPDATE actions. Implementing the move as
+			// DELETE + INSERT would fire ON DELETE actions instead and drop children.
+			updates := make(storage.Row, len(schema.Columns))
+			for _, col := range schema.Columns {
+				if v, ok := lookupRowValue(plan.row, col.Name); ok {
+					updates[col.Name] = v
 				}
 			}
+			_, updated, uerr := e.session.UpdateByPK(tableName, plan.oldPK, func(storage.Row) (storage.Row, error) {
+				return updates, nil
+			})
+			if uerr != nil {
+				return uerr
+			}
+			if !updated {
+				continue
+			}
 			count++
 			if len(stmt.Returning) > 0 {
 				if stored := e.fetchByPK(tableName, newPK); stored != nil {

pkg/executor/foreign_key_cascade_test.go

@@ -442,7 +442,7 @@ func TestForeignKeyCascadeInSQLiteCatalog(t *testing.T) {
 // The capability document advertises the supported referential actions.
 func TestForeignKeyCascadeCapability(t *testing.T) {
 	notes := capabilities.Current().Constraints.ForeignKey.Notes
-	for _, want := range []string{"CASCADE", "SET NULL", "SET DEFAULT", "immediately"} {
+	for _, want := range []string{"CASCADE", "SET NULL", "SET DEFAULT", "autocommit", "no later than COMMIT", "O(FK component)"} {
 		if !strings.Contains(notes, want) {
 			t.Fatalf("foreign_key capability notes = %q, want %q", notes, want)
 		}
@@ -617,3 +617,81 @@ func TestForeignKeyUpdateCascadeToNull(t *testing.T) {
 		t.Fatalf("child.code after cascade to NULL = %v, want NULL", res.Rows[0][0])
 	}
 }
+
+// UPDATE ... FROM that moves a parent key must apply ON UPDATE actions like a
+// plain UPDATE. The executor used to implement the move as DELETE + INSERT,
+// which fired ON DELETE CASCADE and silently dropped the children.
+func TestForeignKeyUpdateFromPrimaryKeyMoveCascades(t *testing.T) {
+	for _, tc := range []struct {
+		name       string
+		constraint string
+	}{
+		{name: "on delete and update cascade", constraint: "ON DELETE CASCADE ON UPDATE CASCADE"},
+		{name: "on update cascade only", constraint: "ON UPDATE CASCADE"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			_, schema, table := newTestDB(t)
+			e := newExec(schema, table)
+			execMust(t, e, "CREATE TABLE parent (id INTEGER PRIMARY KEY)")
+			execMust(t, e, "CREATE TABLE child (id INTEGER PRIMARY KEY, parent_id INTEGER REFERENCES parent(id) "+tc.constraint+")")
+			execMust(t, e, "CREATE TABLE src (id INTEGER PRIMARY KEY, new_id INTEGER)")
+			execMust(t, e, "INSERT INTO parent (id) VALUES (1)")
+			execMust(t, e, "INSERT INTO child (id, parent_id) VALUES (10, 1)")
+			execMust(t, e, "INSERT INTO src (id, new_id) VALUES (1, 2)")
+
+			res := execMust(t, e, "UPDATE parent SET id = src.new_id FROM src WHERE src.id = parent.id")
+			if res.RowsAffected != 1 {
+				t.Fatalf("rows affected = %d, want 1", res.RowsAffected)
+			}
+			if got := countRows(t, e, "child"); got != 1 {
+				t.Fatalf("child rows after UPDATE ... FROM key move = %d, want 1 (children were deleted)", got)
+			}
+			res = execMust(t, e, "SELECT id, parent_id FROM child")
+			if res.Rows[0][0] != int64(10) || res.Rows[0][1] != int64(2) {
+				t.Fatalf("child after UPDATE ... FROM key move = %v, want (10, 2)", res.Rows)
+			}
+			if res := execMust(t, e, "SELECT id FROM parent"); res.RowCount != 1 || res.Rows[0][0] != int64(2) {
+				t.Fatalf("parent after UPDATE ... FROM key move = %v, want id 2", res.Rows)
+			}
+		})
+	}
+}
+
+// A cascade failure inside an explicit transaction is detected at the statement
+// and rolls back to the statement savepoint, so the transaction can continue.
+// A plain NO ACTION/RESTRICT violation is instead deferred to COMMIT, and the
+// statement itself succeeds.
+func TestForeignKeyCascadeTransactionErrorTiming(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE parent (id INTEGER PRIMARY KEY)")
+	execMust(t, e, "CREATE TABLE required (id INTEGER PRIMARY KEY, parent_id INTEGER NOT NULL REFERENCES parent(id) ON DELETE SET NULL)")
+	execMust(t, e, "CREATE TABLE restricted (id INTEGER PRIMARY KEY, parent_id INTEGER REFERENCES parent(id))")
+	execMust(t, e, "INSERT INTO parent (id) VALUES (1), (2)")
+	execMust(t, e, "INSERT INTO required (id, parent_id) VALUES (10, 1)")
+
+	// SET NULL on a NOT NULL child fails at the statement with 23502.
+	execMust(t, e, "BEGIN")
+	_, err := execSQL(e, "DELETE FROM parent WHERE id = 1")
+	requireNotNullViolation(t, err)
+	// The failed cascade rolled back to the statement savepoint: the parent and
+	// child are still visible to the transaction and the transaction is usable.
+	if res := execMust(t, e, "SELECT count(*) FROM parent"); res.Rows[0][0] != int64(2) {
+		t.Fatalf("parent rows after statement rollback = %v, want 2", res.Rows)
+	}
+	if res := execMust(t, e, "SELECT parent_id FROM required WHERE id = 10"); res.Rows[0][0] != int64(1) {
+		t.Fatalf("required.parent_id after statement rollback = %v, want 1", res.Rows[0][0])
+	}
+	execMust(t, e, "ROLLBACK")
+
+	// A NO ACTION/RESTRICT violation is deferred to COMMIT: the DELETE succeeds
+	// as a statement and the transaction fails on commit.
+	execMust(t, e, "INSERT INTO restricted (id, parent_id) VALUES (20, 2)")
+	execMust(t, e, "BEGIN")
+	execMust(t, e, "DELETE FROM parent WHERE id = 2")
+	_, err = execSQL(e, "COMMIT")
+	requireForeignKeyViolation(t, err)
+	if res := execMust(t, e, "SELECT count(*) FROM parent"); res.Rows[0][0] != int64(2) {
+		t.Fatalf("parent rows after rejected commit = %v, want 2", res.Rows)
+	}
+}

pkg/storage/foreign_key_cascade.go

@@ -80,6 +80,55 @@ func (m *SchemaManager) HasCascadingReference(table string, forDelete bool) (boo
 	return false, nil
 }
 
+// beginCascadingPointWrite decides whether an autocommit point write must be
+// staged so its cascades publish with the parent change, and starts that
+// implicit transaction when it is needed.
+//
+// A table outside the foreign-key graph keeps the point-write fast path and
+// never touches the gate. For a participating table the database-wide
+// foreign-key write gate is taken before the cascade classification and held
+// across Begin and the caller's staging. CREATE TABLE ... REFERENCES takes the
+// same unconditional gate, so a constraint cannot be published between deciding
+// that the write is not cascading and opening the transaction that would have
+// applied its actions. The returned release function must be called before
+// Commit, which acquires the same non-reentrant gate.
+//
+// A table gains its first constraint only together with a new child table,
+// which is created empty under the gate and whose rows take the gate on insert.
+// If the participant probe above goes stale, the non-cascading point path
+// re-reads the catalog under the gate and rejects the write with 23503 instead
+// of ignoring the new constraint.
+//
+// The gate is held while Begin and the caller's staging take the session lock.
+// That order cannot invert with Commit: a Session serves one connection and one
+// statement at a time, so Commit on this Session runs after the staging path
+// releases the gate, while other sessions hold their own lock and contend only
+// on the shared gate.
+func (s *Session) beginCascadingPointWrite(table string, forDelete bool) (bool, func(), error) {
+	participates, err := s.schema.ForeignKeyParticipants(table)
+	if err != nil {
+		return false, nil, err
+	}
+	if !participates {
+		return false, func() {}, nil
+	}
+	unlockFK := s.table.LockForeignKeyWriteGate()
+	cascades, err := s.schema.HasCascadingReference(table, forDelete)
+	if err != nil {
+		unlockFK()
+		return false, nil, err
+	}
+	if !cascades {
+		unlockFK()
+		return false, func() {}, nil
+	}
+	if err := s.Begin(); err != nil {
+		unlockFK()
+		return false, nil, err
+	}
+	return true, unlockFK, nil
+}
+
 // foreignKeyRawValues returns the unformatted values a row holds for the given
 // columns, and reports false when a column is missing. A NULL component is
 // returned as nil: on a parent-side update the cascading action copies the new

pkg/storage/tx.go

@@ -786,36 +786,28 @@ func (s *Session) UpdateByPK(table, pk string, updateFn func(Row) (Row, error))
 	s.mu.Lock()
 	if !s.inTx {
 		s.mu.Unlock()
-		cascades, err := s.schema.HasCascadingReference(table, false)
+		cascading, unlockFK, err := s.beginCascadingPointWrite(table, false)
 		if err != nil {
 			return nil, false, err
 		}
-		if !cascades {
+		if !cascading {
 			return s.table.UpdateByPK(table, pk, updateFn)
 		}
-		return s.updateByPKImplicit(table, pk, updateFn)
+		oldRow, updated, err := s.updateByPKStaged(table, pk, updateFn)
+		unlockFK()
+		if err != nil {
+			_ = s.Rollback()
+			return nil, false, err
+		}
+		if err := s.Commit(); err != nil {
+			return nil, false, err
+		}
+		return oldRow, updated, nil
 	}
 	defer s.mu.Unlock()
 	return s.updateByPKLocked(table, pk, updateFn)
 }
 
-// updateByPKImplicit runs a cascading point update inside an implicit
-// transaction, so a failed cascade leaves no durable parent change behind.
-func (s *Session) updateByPKImplicit(table, pk string, updateFn func(Row) (Row, error)) (Row, bool, error) {
-	if err := s.Begin(); err != nil {
-		return nil, false, err
-	}
-	oldRow, updated, err := s.updateByPKStaged(table, pk, updateFn)
-	if err != nil {
-		_ = s.Rollback()
-		return nil, false, err
-	}
-	if err := s.Commit(); err != nil {
-		return nil, false, err
-	}
-	return oldRow, updated, nil
-}
-
 func (s *Session) updateByPKStaged(table, pk string, updateFn func(Row) (Row, error)) (Row, bool, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -881,34 +873,28 @@ func (s *Session) DeleteByPK(table, pk string) (Row, bool, error) {
 	s.mu.Lock()
 	if !s.inTx {
 		s.mu.Unlock()
-		cascades, err := s.schema.HasCascadingReference(table, true)
+		cascading, unlockFK, err := s.beginCascadingPointWrite(table, true)
 		if err != nil {
 			return nil, false, err
 		}
-		if !cascades {
+		if !cascading {
 			return s.table.DeleteByPK(table, pk)
 		}
-		return s.deleteByPKImplicit(table, pk)
+		row, deleted, err := s.deleteByPKStaged(table, pk)
+		unlockFK()
+		if err != nil {
+			_ = s.Rollback()
+			return nil, false, err
+		}
+		if err := s.Commit(); err != nil {
+			return nil, false, err
+		}
+		return row, deleted, nil
 	}
 	defer s.mu.Unlock()
 	return s.deleteByPKLocked(table, pk)
 }
 
-func (s *Session) deleteByPKImplicit(table, pk string) (Row, bool, error) {
-	if err := s.Begin(); err != nil {
-		return nil, false, err
-	}
-	row, deleted, err := s.deleteByPKStaged(table, pk)
-	if err != nil {
-		_ = s.Rollback()
-		return nil, false, err
-	}
-	if err := s.Commit(); err != nil {
-		return nil, false, err
-	}
-	return row, deleted, nil
-}
-
 func (s *Session) deleteByPKStaged(table, pk string) (Row, bool, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()