Skip to content

box: disallow alter of primary index with dependent secondary#11536

Merged
locker merged 1 commit intotarantool:masterfrom
drewdzzz:alter_pk_crash
May 30, 2025
Merged

box: disallow alter of primary index with dependent secondary#11536
locker merged 1 commit intotarantool:masterfrom
drewdzzz:alter_pk_crash

Conversation

@drewdzzz
Copy link
Contributor

Rebuild of several indexes in one statement is broken because we need to maintain consistency of built indexes while building the next ones, and we don't do that. Such rebuilds can happen only when primary index is altered and all non-unique and nullable indexes need to be rebuilt as well. Let's simply forbid such alters.

We could forbid any alter of primary index, and the patch would be even a bit simpler, but let's allow to alter primary in a space without dependent secondary indexes - it seems to work fine, and people would use it, at least someone reported this problem, hence, someone actually tried to alter primary.

Closes #10951

@drewdzzz drewdzzz requested a review from a team as a code owner May 27, 2025 15:41
@drewdzzz drewdzzz force-pushed the alter_pk_crash branch 2 times, most recently from dc865dd to 36af048 Compare May 27, 2025 15:48
@coveralls
Copy link

coveralls commented May 27, 2025

Coverage Status

coverage: 87.528% (+0.02%) from 87.509%
when pulling 5ffb7bd on drewdzzz:alter_pk_crash
into 30d20ee
on tarantool:master
.

@drewdzzz drewdzzz requested review from locker and nshy May 27, 2025 16:21
@locker locker assigned drewdzzz and unassigned locker May 28, 2025
@nshy nshy removed their assignment May 28, 2025
@drewdzzz drewdzzz force-pushed the alter_pk_crash branch 2 times, most recently from 773072b to 9af1696 Compare May 28, 2025 15:39
@drewdzzz drewdzzz changed the title memtx: disallow alter of primary index with dependent secondary box: disallow alter of primary index with dependent secondary May 28, 2025
@drewdzzz
Copy link
Contributor Author

Note that I use box section for the commit title now (was memtx before). In the changelog entry, I still use memtx section because it describes behavior of memtx, and in other engines the bug is barely reproducible.

@drewdzzz drewdzzz requested review from locker and nshy May 28, 2025 15:55
@drewdzzz drewdzzz assigned locker and nshy and unassigned drewdzzz May 28, 2025
@locker locker removed their assignment May 28, 2025
@nshy nshy assigned drewdzzz and unassigned nshy May 29, 2025
Rebuild of several indexes in one statement is broken because we need
to maintain consistency of built indexes while building the next ones,
and we don't do that. Such rebuilds can happen only when primary index
is altered and all non-unique and nullable indexes need to be rebuilt as
well. Let's simply forbid such alters.

We could forbid any alter of primary index, and the patch would be even
a bit simpler, but let's allow to alter primary in a space without
dependent secondary indexes - it seems to work fine, and people would
use it, at least someone reported this problem, hence, someone actually
tried to alter primary.

Closes tarantool#10951

NO_DOC=bugfix
@drewdzzz drewdzzz added the full-ci Enables all tests for a pull request label May 29, 2025
@drewdzzz drewdzzz assigned locker and unassigned drewdzzz May 30, 2025
@locker locker merged commit f34d0bf into tarantool:master May 30, 2025
76 of 86 checks passed
@locker
Copy link
Member

locker commented May 30, 2025

Cherry-picked to 3.2, 3.3, 3.4.

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

Labels

full-ci Enables all tests for a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Alter of PK while having non-unique indexes doesn't work properly

5 participants