sql: clean up column deps during rollback of CREATE TABLE in ADD state

[shghasemi]

Previously, when a CREATE TABLE with user-defined type columns or sequence defaults was rolled back (e.g., due to a schema change job failure), the table's ID was left in the type descriptor's ReferencingDescriptorIDs and the sequence's DependedOnBy. After GC removed the table descriptor, these descriptors would have orphaned back-references to a non-existent descriptor, causing them to appear in crdb_internal.invalid_objects.

The fix adds a dropColumnDeps method that iterates over the table's columns, finds all referenced user-defined type descriptors and sequences, and removes the table's ID from their back-references. This is called during rollbackSchemaChange alongside the existing dropFKDeps.

Fixes: #165836

Release note (bug fix): Fixed a bug where rolling back a CREATE TABLE that referenced user-defined types or sequences would leave orphaned back-references on the type and sequence descriptors, causing them to appear in crdb_internal.invalid_objects after the table was GC'd.

[trunk-io]

😎 Merged successfully - details.

[cockroach-teamcity]

This change is 

[rafiss]

nice work so far!

can we also test function references? they currently are not cleaned up in dropColumnDeps. If a CREATE TABLE with a column that uses a UDF in a default expression is rolled back, will the function descriptor's back-reference be orphaned?

this is a pre-existing gap (the original dropViewDeps didn't handle it either), but since this PR is explicitly about cleaning up column deps during rollback, it would be the right time to address it.

@rafiss made 5 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on shghasemi).

---

pkg/sql/schema_changer.go line 1211 at r1 (raw file):

	viewDesc.DependsOn = nil
	// If anything depends on this table clean up references from that object as well.
	DependedOnBy := append([]descpb.TableDescriptor_Reference(nil), viewDesc.DependedOnBy...)

nit: don't use an uppercase name for a local variable.

---

pkg/sql/schema_changer.go line 1257 at r1 (raw file):

				if err := descsCol.WriteDescToBatch(ctx, false /* kvTrace */, typeDesc, b); err != nil {
					log.Dev.Warningf(ctx, "error removing dependency from type ID %d during rollback: %v", id, err)
					continue

i think we should keep this return err. the existing dropFKDeps method does return WriteDescToBatch errors, and i think we should keep the convention that batch-write failures should propagate.

the continue pattern is appropriate for descriptor resolution errors (where the descriptor may not exist), but not for write failures.

---

pkg/sql/schema_changer.go line 1280 at r1 (raw file):

			if err := descsCol.WriteDescToBatch(ctx, false /* kvTrace*/, seqDesc, b); err != nil {
				log.Dev.Warningf(ctx, "error removing dependency from sequence ID %d during rollback: %v", seqID, err)
				continue

let's also return the error here.

---

pkg/sql/schema_changer.go line 1406 at r1 (raw file):

		}

		if err := sc.dropColumnDeps(ctx, txn.Descriptors(), txn.KV(), b, scTable); err != nil {

this function will now get called twice for views (since dropViewDeps also calls it). we should either add a if !scTable.IsView() check before this call, or it might even be more clear to remove the call to dropColumnDeps from the dropViewDeps function.

[shghasemi]

I confirmed that this was an existing issue with the test and fixed it in dropColumnDeps.

@shghasemi made 4 comments and resolved 1 discussion.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on rafiss and shghasemi).

---

pkg/sql/schema_changer.go line 1257 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i think we should keep this return err. the existing dropFKDeps method does return WriteDescToBatch errors, and i think we should keep the convention that batch-write failures should propagate.

the continue pattern is appropriate for descriptor resolution errors (where the descriptor may not exist), but not for write failures.

Done.

---

pkg/sql/schema_changer.go line 1280 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

let's also return the error here.

Done.

---

pkg/sql/schema_changer.go line 1406 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

this function will now get called twice for views (since dropViewDeps also calls it). we should either add a if !scTable.IsView() check before this call, or it might even be more clear to remove the call to dropColumnDeps from the dropViewDeps function.

You're right. Fixed. We can't remove it from dropViewDeps because that function runs recursively to handle views that reference another view. I added the check to avoid calling it twice.

[blathers-crl]

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

[rafiss]

nice work! thanks for the new test

@rafiss made 1 comment.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on shghasemi).

[shghasemi]

TFTR Rafi!

/trunk merge

[blathers-crl]

⚠️ End-of-Life Version Warning

You have requested backports to the following End-of-Life (EOL) versions:

• 26.2

While Blathers will still create these backport PRs, please verify that backporting to EOL versions is intentional and appropriate. EOL versions no longer receive maintenance updates according to our support policy.

The backport PRs will also include this warning.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[blathers-crl]

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.

---

Issue #165836: branch-release-25.4, branch-release-25.4.7-rc.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

merge conflict cherry-picking 057cfcc to blathers/backport-release-25.4-166015

Backport to branch 25.4.x failed. See errors above.

---

merge conflict cherry-picking 057cfcc to blathers/backport-release-25.4.7-rc-166015

Backport to branch 25.4.7-rc failed. See errors above.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[rafiss]

blathers backport 25.4 25.4.7-rc