-
Notifications
You must be signed in to change notification settings - Fork 4.1k
sql: alter pk using subset of old pk key columns should always rewrite secondary indexes #84040
Description
Describe the problem
Currently when doing ALTER PRIMARY kEY, we have smart logic to determine whether to rewrite secondary index or not. Simply put, the logic is based on the rule of how we encode secondary index key/value pair and if the secondary index key/value pair has enough information to construct a primary key to fetch more data. There is one corner case, when we chose not to rewrite, secondary index still has references to columns from the old primary key in suffixKeyColumns that's not part of new primary key. This is problematic because user could drop the old primary key column since they think they're not needed any more, and that would cause the secondary index to be dropped silently (see this issue)
See the repro below for details about the specific issue. This happens in all versions.
To Reproduce
create table t (
a int not null,
b int not null,
c int not null,
primary key (a, b),
unique index uniq_idx (c) -- this index is unique, and all its key columns are not nullable.
-- based on crdb's encoding theory, the index value contains columns "a" and "b"
-- in the index's suffix columns. the index key only contains column "c"
);
-- now alter primary key to a subset of old primary key columns
alter table t alter primary key using columns (a); -- new primary key col "a" is a subset of "a, b".
-- at this moment `uniq_idx`'s suffix columns still contain "a" and "b"
-- because it was not rewritten. This is because our logic think that
-- `uniq_idx` has enough information to infer a primary key. And because
-- it's an unique key with only not nullable columns, its index key does not
-- contain any primary key column, so we chose not to rewrite this index.
-- now let's drop column `b`, because we think it's not needed...
-- note that `b` is not in primary key and `uniq_idx` when we does a SHOW CREATE TABLE.
alter table t drop column b;
-- boom....`uniq_idx` is dropped because we think that column "b" is referenced by it :(Desired Behavior
We need to rewrite the secondary index in this case even we have enough information to infer a primary key.