Skip to content

Commit 450be14

Browse files
committed
Bug#36658450 Crash when drop and add primary key with desc
Description: ============ Dropping a primary key and adding a new auto-increment column as a primary key in descending order using the "inplace" algorithm fails. Analysis: ========= Dropping an existing primary key and adding a new auto-increment key in descending order requires arranging the records in reverse order, which necessitates a file sort. However, this scenario was not detected in the method innobase_pk_order_preserved(), causing it to return false. As a result, the ALTER INPLACE operation, which calls this method, skips the file sort. Instead, it processes the primary key as usual in batches, a method known as bulk mode. In bulk mode, records are inserted into a sort buffer (in descending order in this case). When the sort buffer becomes full, records are directly inserted into the B-tree. Consider a case where we have 2000 records, and the sort buffer can hold 1000 records in a batch: Batch #1 inserted: Records 1000 to 1 (in descending order) Batch #2 inserted: Records 2000 to 1001 (in descending order) If the records from both batches happen to be in the same page, the record order is violated. It's important to note that this record order violation would still exist even if the sort buffer were skipped when file sort was skipped. Therefore, enabling file sort is essential to ensure correct record order across batches. Fix: ==== Enable file sort when add autoinc descending. This patch is based on the contribution from Shaohua Wang at Alibaba Group. We thank you for contributing to MySQL. Change-Id: I398173bbd27db7f5e29218d217bf11c30297c242
1 parent f682827 commit 450be14

File tree

1 file changed

+14
-5
lines changed

1 file changed

+14
-5
lines changed

storage/innobase/handler/handler0alter.cc

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3551,7 +3551,8 @@ static inline bool innobase_pk_col_is_existing(const ulint new_col_no,
35513551
}
35523552

35533553
/** Determine whether both the indexes have same set of primary key
3554-
fields arranged in the same order.
3554+
fields arranged in the same order. If so, there is no need to do the
3555+
external sorting of primary key fields.
35553556
35563557
Rules when we cannot skip sorting:
35573558
(1) Removing existing PK columns somewhere else than at the end of the PK;
@@ -3562,14 +3563,16 @@ columns are removed from the PK;
35623563
follows rule(1), Increasing the prefix length just like adding existing
35633564
PK columns follows rule(2);
35643565
(5) Changing the ascending order of the existing PK columns.
3566+
(6) Adding a new auto increment column with descending order in PK.
35653567
@param[in] col_map mapping of old column numbers to new ones
35663568
@param[in] old_clust_index index to be compared
35673569
@param[in] new_clust_index index to be compared
3570+
@param[in] add_autoinc added AUTO_INCREMENT column position
35683571
@retval true if both indexes have same order.
35693572
@retval false. */
35703573
[[nodiscard]] static bool innobase_pk_order_preserved(
35713574
const ulint *col_map, const dict_index_t *old_clust_index,
3572-
const dict_index_t *new_clust_index) {
3575+
const dict_index_t *new_clust_index, ulint add_autoinc) {
35733576
ulint old_n_uniq = dict_index_get_n_ordering_defined_by_user(old_clust_index);
35743577
ulint new_n_uniq = dict_index_get_n_ordering_defined_by_user(new_clust_index);
35753578

@@ -3621,7 +3624,13 @@ PK columns follows rule(2);
36213624
} else if (innobase_pk_col_is_existing(new_col_no, col_map, old_n_cols)) {
36223625
new_field_order = old_n_uniq + existing_field_count++;
36233626
} else {
3624-
/* Skip newly added column. */
3627+
/* Skip newly added column except descending auto increment column */
3628+
if (add_autoinc == new_col_no &&
3629+
!new_clust_index->fields[new_field].is_ascending) {
3630+
/* Descending needs sort */
3631+
return (false);
3632+
}
3633+
36253634
continue;
36263635
}
36273636

@@ -4947,8 +4956,8 @@ template <typename Table>
49474956
if (new_clustered) {
49484957
dict_index_t *clust_index = user_table->first_index();
49494958
dict_index_t *new_clust_index = ctx->new_table->first_index();
4950-
ctx->skip_pk_sort =
4951-
innobase_pk_order_preserved(ctx->col_map, clust_index, new_clust_index);
4959+
ctx->skip_pk_sort = innobase_pk_order_preserved(
4960+
ctx->col_map, clust_index, new_clust_index, ctx->add_autoinc);
49524961

49534962
DBUG_EXECUTE_IF("innodb_alter_table_pk_assert_no_sort",
49544963
assert(ctx->skip_pk_sort););

0 commit comments

Comments
 (0)