Description:
The case what I want to say, is the same to bug#76895 which was classified as documented limitation of online ddl.
My point is the duplicate key error of alter report a incorrect key name. It alway report PRIMARY KEY as the duplicate key, but in this case the PK will never duplicate.
The root cause of the incorrect key name is while apply online log of alter, we do not record the error key num, so we always report PK.
This error may confused users, and can not help user to find out the real duplicate key.
At last, I still hope to be able to eliminate this limitation. Is there any plan? or I will eliminate this limitation in some common type of online ddl.
How to repeat:
--source include/have_innodb.inc
--source include/have_debug_sync.inc
#
# increment data test(has apply log)
#
#double column
create table t1 (id int auto_increment primary key, a int, unique key uk(a)) engine = innodb;
insert into t1 select 1, 1;
insert into t1 select 2, 2;
connect (con1,localhost,root,,);
connection con1;
SET DEBUG_SYNC = 'row_log_table_apply1_before WAIT_FOR continue';
--send alter table t1 add b int, ALGORITHM=inplace;
connection default;
--sleep 1
--error ER_DUP_ENTRY
insert into t1 select NULL, 1;
SET DEBUG_SYNC = 'now SIGNAL continue';
connection con1;
--error ER_DUP_ENTRY
--reap
connection default;
SET DEBUG_SYNC = 'now SIGNAL nothing';
connection con1;
SET DEBUG_SYNC = 'row_log_table_apply1_before WAIT_FOR continue';
--send alter table t1 add b int, ALGORITHM=inplace;
connection default;
--sleep 1
--error ER_DUP_ENTRY
update t1 set a=1 where id=2;
SET DEBUG_SYNC = 'now SIGNAL continue';
connection con1;
--error ER_DUP_ENTRY
--reap
connection default;
SET DEBUG_SYNC = 'now SIGNAL nothing';
#clean up
drop table t1;
Suggested fix:
The patch to report the correct key name.
diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
index 9cf2ac2..5138c96 100644
--- a/storage/innobase/handler/handler0alter.cc
+++ b/storage/innobase/handler/handler0alter.cc
@@ -4032,7 +4032,8 @@ oom:
if (error == DB_SUCCESS && ctx->online && ctx->need_rebuild()) {
DEBUG_SYNC_C("row_log_table_apply1_before");
error = row_log_table_apply(
- ctx->thr, prebuilt->table, altered_table);
+ ctx->thr, prebuilt->table, altered_table,
+ &prebuilt->trx->error_key_num);
}
DEBUG_SYNC_C("inplace_after_index_build");
@@ -4920,7 +4921,8 @@ commit_try_rebuild(
if (ctx->online) {
DEBUG_SYNC_C("row_log_table_apply2_before");
error = row_log_table_apply(
- ctx->thr, user_table, altered_table);
+ ctx->thr, user_table, altered_table,
+ &(thr_get_trx(ctx->thr)->error_key_num));
ulint err_key = thr_get_trx(ctx->thr)->error_key_num;
switch (error) {
diff --git a/storage/innobase/include/row0log.h b/storage/innobase/include/row0log.h
index 62715fe..e4ac5b4 100644
--- a/storage/innobase/include/row0log.h
+++ b/storage/innobase/include/row0log.h
@@ -203,7 +203,9 @@ row_log_table_apply(
que_thr_t* thr, /*!< in: query graph */
dict_table_t* old_table,
/*!< in: old table */
- struct TABLE* table) /*!< in/out: MySQL table
+ struct TABLE* table, /*!< in/out: MySQL table
+ (for reporting duplicates) */
+ ulint* error_key_num) /*!< in/out: dup key num
(for reporting duplicates) */
__attribute__((nonnull, warn_unused_result));
diff --git a/storage/innobase/include/row0merge.h b/storage/innobase/include/row0merge.h
index 598d772..af33cd4 100644
--- a/storage/innobase/include/row0merge.h
+++ b/storage/innobase/include/row0merge.h
@@ -117,6 +117,9 @@ struct row_merge_dup_t {
(index->table), or NULL if not
rebuilding table */
ulint n_dup; /*!< number of duplicates */
+ ulint* error_key_num; /*!< if the index creation fails to a
+ duplicate key error, a mysql key
+ number of that index is stored here */
};
/*************************************************************//**
diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/row0log.cc
index cf072f5..e29d17e 100644
--- a/storage/innobase/row/row0log.cc
+++ b/storage/innobase/row/row0log.cc
@@ -1471,6 +1471,7 @@ row_log_table_apply_insert_low(
dict_index_t* index = dict_table_get_first_index(log->table);
ulint page_no = ULINT_UNDEFINED;
ullint modify_clock = ULLINT_UNDEFINED;
+ ulint dup_key_num = 0;
ut_ad(dtuple_validate(row));
ut_ad(trx_id);
@@ -1510,6 +1511,7 @@ row_log_table_apply_insert_low(
if (!(index = dict_table_get_next_index(index))) {
break;
}
+ dup_key_num ++;
if (index->type & DICT_FTS) {
continue;
@@ -1520,6 +1522,10 @@ row_log_table_apply_insert_low(
flags, BTR_MODIFY_TREE,
index, offsets_heap, heap, entry, trx_id, thr,
&page_no, &modify_clock);
+
+ if (error == DB_DUPLICATE_KEY)
+ *(dup->error_key_num) = dup_key_num;
+
} while (error == DB_SUCCESS);
return(error);
@@ -1827,6 +1833,7 @@ row_log_table_apply_update(
mtr_t mtr;
btr_pcur_t pcur;
dberr_t error;
+ ulint dup_key_num = 0;
ut_ad(dtuple_get_n_fields_cmp(old_pk)
== dict_index_get_n_unique(index));
@@ -2102,6 +2109,8 @@ func_exit_committed:
break;
}
+ dup_key_num ++;
+
if (index->type & DICT_FTS) {
continue;
}
@@ -2148,6 +2157,9 @@ func_exit_committed:
entry, trx_id, thr,
&page_no, &modify_clock);
+ if (error == DB_DUPLICATE_KEY)
+ *(dup->error_key_num) = dup_key_num;
+
mtr_start(&mtr);
}
@@ -2802,7 +2814,9 @@ row_log_table_apply(
que_thr_t* thr, /*!< in: query graph */
dict_table_t* old_table,
/*!< in: old table */
- struct TABLE* table) /*!< in/out: MySQL table
+ struct TABLE* table, /*!< in/out: MySQL table
+ (for reporting duplicates) */
+ ulint* error_key_num) /*!< in/out: dup key num
(for reporting duplicates) */
{
dberr_t error;
@@ -2828,7 +2842,8 @@ row_log_table_apply(
} else {
row_merge_dup_t dup = {
clust_index, table,
- clust_index->online_log->col_map, 0
+ clust_index->online_log->col_map, 0,
+ error_key_num
};
error = row_log_table_apply_ops(thr, &dup);
@@ -3629,7 +3644,7 @@ row_log_apply(
{
dberr_t error;
row_log_t* log;
- row_merge_dup_t dup = { index, table, NULL, 0 };
+ row_merge_dup_t dup = { index, table, NULL, 0 , NULL};
DBUG_ENTER("row_log_apply");
ut_ad(dict_index_is_online_ddl(index));
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
index 3f7ccf4..28c657a 100644
--- a/storage/innobase/row/row0merge.cc
+++ b/storage/innobase/row/row0merge.cc
@@ -1340,7 +1340,7 @@ row_merge_read_clustered_index(
merge_buf = static_cast<row_merge_buf_t**>(
mem_alloc(n_index * sizeof *merge_buf));
- row_merge_dup_t clust_dup = {index[0], table, col_map, 0};
+ row_merge_dup_t clust_dup = {index[0], table, col_map, 0, NULL};
dfield_t* prev_fields;
const ulint n_uniq = dict_index_get_n_unique(index[0]);
@@ -1852,7 +1852,7 @@ write_buffers:
}
} else if (dict_index_is_unique(buf->index)) {
row_merge_dup_t dup = {
- buf->index, table, col_map, 0};
+ buf->index, table, col_map, 0, NULL};
row_merge_buf_sort(buf, &dup);
@@ -3954,7 +3954,7 @@ wait_again:
} else if (merge_files[i].fd >= 0) {
row_merge_dup_t dup = {
- sort_idx, table, col_map, 0};
+ sort_idx, table, col_map, 0, NULL};
error = row_merge_sort(
trx, &dup, &merge_files[i],
