Skip to content

sql: fix comments and error handling in temp schema cleanup#167441

Open
rafiss wants to merge 1 commit intocockroachdb:masterfrom
rafiss:fix-temp-cleanup-nits
Open

sql: fix comments and error handling in temp schema cleanup#167441
rafiss wants to merge 1 commit intocockroachdb:masterfrom
rafiss:fix-temp-cleanup-nits

Conversation

@rafiss
Copy link
Copy Markdown
Collaborator

@rafiss rafiss commented Apr 2, 2026

Add a few cleanups to follow onto #166915:

  • Fix tblDescsByID field comment which incorrectly said "maps sequence IDs" when it actually maps all object IDs.
  • Propagate the error from ForEachDescriptor instead of discarding it.
  • Restore the removed comment explaining why only unowned sequences are explicitly dropped (owned ones are dropped via CASCADE).
  • Add comment explaining why IF EXISTS is needed in DROP statements.
  • Handle ErrDescriptorNotFound in cleanupTempSequenceDeps when a dependent permanent table is dropped between Phase 1 and Phase 2, treating it as a no-op instead of triggering retry loops.
  • Fix two stale "preHook" references in test comments.

Informs: #166815
Release note: None

Add a few cleanups to follow onto cockroachdb#166915:

- Fix `tblDescsByID` field comment which incorrectly said "maps sequence
  IDs" when it actually maps all object IDs.
- Propagate the error from `ForEachDescriptor` instead of discarding it.
- Restore the removed comment explaining why only unowned sequences are
  explicitly dropped (owned ones are dropped via CASCADE).
- Add comment explaining why `IF EXISTS` is needed in DROP statements.
- Handle `ErrDescriptorNotFound` in `cleanupTempSequenceDeps` when a
  dependent permanent table is dropped between Phase 1 and Phase 2,
  treating it as a no-op instead of triggering expensive retry loops.
- Fix two stale "preHook" references in test comments.

Informs: cockroachdb#166915
Release note: None

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
@rafiss rafiss requested review from eric-alton and fqazi April 2, 2026 21:21
@trunk-io
Copy link
Copy Markdown
Contributor

trunk-io bot commented Apr 2, 2026

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants