sql: don't store primary key col when rewriting secondary indexes during ALTER PK #115214
Conversation
…ing ALTER PK Release note (bug fix): Previously, if we have a table with secondary indexes that store some columns `col` via the `STORING` clause, and we alter primary key to `col` on this table, then we'd end up with incorrect secondary index where it continues to have the `STORING` clause, even though that column is already in the primary key and we don't allow creating a secondary index storing any primary key columns. This commit fixes that such that after the ALTER PK, the `STORING` clause is dropped on those secondary indexes.
|
This change is |
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @Xiang-Gu)
-- commits line 7 at r1:
Out of curiosity does that mean we store columns twice when we do inserts in both the key / value? If we aren't doing that maybe this warrants having a automated repair + validation as a separate PR (that shouldn't be back ported.
I assume what you meant is after this fix, the secondary index will have
I do think it's a good idea to add a validation for it (basically, we'd assert that "for any secondary index, any primary key column will either be its key column or its key suffix column", and that "for any secondary index, no stored columns should be in primary key columns". |
|
TFTR! bors r+ |
|
Build succeeded: |
Xiang-Gu
deleted the
bug/alter-pk-to-a-stored-column-by-an-secondary-index
branch
Previously, we encountered bugs (cockroachdb#115214) that lead to corrupted index descriptor where it stores a primary key column. This commit added a validation logic to ensure such corruption is caught. Release note: None
Previously, we encountered bugs (cockroachdb#115214) that lead to corrupted index descriptor where it stores a primary key column. This commit added a validation logic to ensure such corruption is caught. This commit also comes with a test to ensure the validation is silenced in a mixed version state so that existing corruption won't disturb any SQL workload but cluster version upgrade is indeed blocked. Release note: None
115048: roachtest : add --use-spot option r=BabuSrithar a=BabuSrithar This options enables use of SpotVM to run tests, If the provider does not support spotVM, it will be ignored Epic: none Release note: None 115558: tabledesc: validate that primary key columns are not stored in an index r=Xiang-Gu a=Xiang-Gu Fixes #115293 Previously, we encountered bugs (#115214) that lead to corrupted index descriptor where it stores a primary key column. This commit added a validation logic to ensure such corruption is caught. Release note: None 116226: kvserver: skip swap voters with non-voters under deadlock r=arulajmani a=kvoli Liveness heartbeats may fail under deadlock builds with large test clusters, causing the test to time out waiting for suspect stores to become eligible targets. Skip `TestReplicateQueueSwapVotersWithNonVoters` under deadlock builds. Release note: None Resolves: #116225 116283: bazel: upgrade `rules_go` to v0.43 r=rail,dt a=rickystewart Also update core dependencies that must be updated for the new `rules_go` version. Informs #99988 Epic: none Release note: None 116285: roachtest: move admission control to storage team r=nicktrav,aadityasondhi a=kvoli Storage now owns AC. Update the `TEAMS.yaml` so roachtest failures are filed under `T-storage`, instead of `T-kv`. Epic: none Release note: None Co-authored-by: babusrithar <[email protected]> Co-authored-by: Xiang Gu <[email protected]> Co-authored-by: Austen McClernon <[email protected]> Co-authored-by: Ricky Stewart <[email protected]>
Fixes #114758
Release note (bug fix): Previously, if we have a table with secondary indexes that store some columns
colvia theSTORINGclause, and we alter primary key tocolon this table, then we'd end up with incorrect secondary index where it continues to have theSTORINGclause, even though that column is already in the primary key and we don't allow creating a secondary index storing any primary key columns. This commit fixes that such that after the ALTER PK, theSTORINGclause is dropped on those secondary indexes.