sql: ALTER PRIMARY KEY rewrites secondary index if new PK subsets old PK#84303
Conversation
|
This change is |
08534aa to
5ccd4ed
Compare
ajwerner
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @Xiang-Gu)
pkg/sql/logictest/testdata/logic_test/alter_primary_key line 1645 at r1 (raw file):
# were rewritten, and hence dropping a column from the old PK columns does not # unexpectedly drop an existing secondary index. subtest regression_#84040
Can you add a test that explicitly tests the hash sharded index case?
Code quote:
# The following regression test makes sure that when the new PK columns
# is a (strict) subset of the old PK columns, all existing secondary indexes
# were rewritten, and hence dropping a column from the old PK columns does not
# unexpectedly drop an existing secondary index.
subtest regression_#84040
6c92b44 to
190246b
Compare
ALTER PRIMARY KEY rewrites secondary index if new PK subsets old PK
Previously, during a `ALTER PRIMARY KEY`, if the new PK columns is a subset of the old PK columns, we won't rewrite existing unique, secondary index. This was inadequate because the user might think this column is not used anywhere and will want to drop it, which will unexpectedly drop the dependent unique index. Release note (bug fix): This PR fixed a bug where, in a `ALTER PRIMARY KEY`, if the new PK columns is a subset of the old PK columns, we will not rewrite existing secondary index, and hence those secondary indexes continue to have some of the old PK columns in their `suffixColumns`. But the user might, reasonably, think those columns are not used anymore and proceed to drop them. The bug then caused those dependent secondary indexes to be dropped, unexpectedly for the user.
190246b to
53bf4d5
Compare
|
bors r+ |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 53bf4d5 to blathers/backport-release-21.2-84303: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 21.2.x failed. See errors above. error creating merge commit from 53bf4d5 to blathers/backport-release-22.1-84303: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
|
@Xiang-Gu don't forget to backport this one manually as well. |
Previously, during a
ALTER PRIMARY KEY, if the new PK columns is asubset of the old PK columns, we won't rewrite existing unique,
secondary index.
This was inadequate because the user might think this column is not used
anywhere and will want to drop it, which will unexpectedly drop the
dependent unique index.
Fixes: #84040
Release note (bug fix): This PR fixed a bug where, in a
ALTER PRIMARY KEY, if the new PK columns is asubset of the old PK columns, we will not rewrite existing secondary
index, and hence those secondary indexes continue to have some of
the old PK columns in their
suffixColumns.But the user might, reasonably, think those columns are not used anymore
and proceed to drop them. The bug then caused those dependent secondary
indexes to be dropped, unexpectedly for the user.