Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: fix LSC that incorrectly missed secondary index rewrite during ALTER PK #114622

Merged

Conversation

Xiang-Gu
Copy link
Contributor

@Xiang-Gu Xiang-Gu commented Nov 16, 2023

Fixes #114436

Release note (bug fix): Previously, when set use_declarative_schema_changer=off
and we attempt to alter primary key on a table that has unique secondary
indexes on new-pk-cols, the unique secondary index would incorrectly
still have old-pk-cols as its keySuffixColumn after the ALTER PK. This
is problematic because a subsequent dropping of the old-pk-cols would
unexpectedly drop those unique secondary indexes as well, even without
CASCADE. This commit fixes that.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

…LTER PK

Fixes cockroachdb#114436

Release note (bug fix): Previously, when set `use_declarative_schema_changer=off`
and we attempt to alter primary key on a table that has unique secondary
indexes on new-pk-cols, the unique secondary index would incorrectly
still have old-pk-cols as its keySuffixColumn after the ALTER PK. This
is problematic because a subsequent dropping of the old-pk-cols would
unexpectedly drop those unique secondary indexes as well, even without
CASCADE. This commit fixes that.
@Xiang-Gu Xiang-Gu force-pushed the bug/alter-pk-legacy-rowid-and-unique-index branch from 5eb6168 to 8f293f6 Compare November 20, 2023 21:01
@Xiang-Gu Xiang-Gu marked this pull request as ready for review November 21, 2023 15:31
@Xiang-Gu Xiang-Gu requested a review from a team as a code owner November 21, 2023 15:31
Copy link
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: Nice find!

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @Xiang-Gu)

@Xiang-Gu
Copy link
Contributor Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 21, 2023

Build succeeded:

@craig craig bot merged commit 88233d4 into cockroachdb:master Nov 21, 2023
8 checks passed
@Xiang-Gu Xiang-Gu deleted the bug/alter-pk-legacy-rowid-and-unique-index branch November 27, 2023 16:28
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.

sql: ALTER PK on table with unique index on new-PK-cols incorrectly left old-PK-cols in index
3 participants