| Bug #115136 | Skip value when add an auto_increment column | ||
|---|---|---|---|
| Submitted: | 27 May 2024 9:01 | Modified: | 27 Mar 9:04 |
| Reporter: | Huaxiong Song (OCA) | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: InnoDB storage engine | Severity: | S2 (Serious) |
| Version: | 8.0,8.4 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[27 May 2024 10:07]
MySQL Verification Team
HI Mr. Song, Thank you for your bug report. However, this is not a bug. Autoincrement keys start from 1, so with ten thousand tuples, its largest value is 10.000. If you take a look at your table and read our Reference Manual, you will see that a result is corret. Not a bug.
[27 May 2024 10:34]
Huaxiong Song
Please look at the definition of the stored procedure. I only have 9999 pieces of data, but the IDs from 1 to 10000 are 10000 pieces of data, right? In fact, we can simply find that a certain piece of data has been skipped. For example, in the test on my server, id = 9533 was skipped. Through this SQL, we can see the following results: MySQL [test]> select * from a; +------+------+-------+ | a | b | id | +------+------+-------+ .... | 9532 | 9532 | 9532 | | 9533 | 9533 | 9534 | | 9534 | 9534 | 9535 | | 9535 | 9535 | 9536 | ... | 9999 | 9999 | 10000 | +------+------+-------+ 9999 rows in set (0.01 sec)
[27 May 2024 10:59]
MySQL Verification Team
Hi Mr. Song, You are quite right. However, this is how InnoDB works with auto-increment keys. It reserves a number of values in advance, before insertion starts. During insertion, it can skip certain values. This is described in our Reference Manual.
[27 May 2024 11:06]
Huaxiong Song
Hi, this is what I see: With innodb_autoinc_lock_mode set to 0 (“traditional”) or 1 (“consecutive”), the auto-increment values generated by any given statement are consecutive, without gaps, because the table-level AUTO-INC lock is held until the end of the statement, and only one such statement can execute at a time.
[28 May 2024 2:31]
Huaxiong Song
Sorry.... which Reference Manual should I refer to? There seems to be no mention in the Reference Manual that data skips will occur when innodb_autoinc_lock_mode=0. Or can you tell me where to introduce it? From my current reading, data reservation and gap should only occur when innodb_autoinc_lock_mode = [1|2].
[28 May 2024 10:36]
MySQL Verification Team
Thank you , Mr. Song, We shall file a Documentation bug for the missing info in the Manual.
[29 May 2024 2:05]
Huaxiong Song
Hi, I think your answer can be further discussed...
First, I want to know if gaps are still possible under innodb_autoinc_lock_mode = 0 (just DDL as described in the test case, no deletion, no update, no rollback), yes or no?
Second, from the code of parallel DDL, the multi-threaded processing of auto-increment values is protected by mutex, and they can only increase sequentially (but gaps actually occur).
```
dberr_t Context::handle_autoinc(const dtuple_t *dtuple) noexcept {
...
mutex_enter(&m_autoinc_mutex);
auto value = m_sequence++;
mutex_exit(&m_autoinc_mutex);
...
}
```
Finally, let's increase the amount of data and execute the test case. It can be found that the intervals of the gap values are fixed. I think these gap values should have increased sequentially. In other words, they are not skipped randomly.
```
call insert_into_tables(100000);
select b from a where b not in (select id from a);
b
7150
14300
21450
28600
35750
42900
50050
57200
64350
71500
78650
85800
92950
```
[29 May 2024 10:24]
MySQL Verification Team
Hi Mr. Song, A documentation bug has been filed. We have added your last comments to it, so we shall see what will happen.
[25 Sep 2024 11:50]
Huaxiong Song
I have further analyzed and tested this issue and I am convinced that this is a bug. This question has nothing to do with 'this is how InnoDB works with auto-increment keys'.
The following is a supplement to the original test case:
After increasing the size of dd_buffer, we can find that the skip value disappears.
# test case
set @ori_val = @@innodb_ddl_buffer_size;
create table a(a int, b int, primary key(a));
delimiter $$;
CREATE PROCEDURE insert_into_tables(IN num INTEGER)
BEGIN
declare x INT;
set x = 1;
while x < num do
INSERT INTO `a` (`a`, `b`) VALUES (x, x);
set x = x + 1;
end while;
end$$
delimiter ;$$
call insert_into_tables(10000);
alter table a add column `id` bigint UNSIGNED NOT NULL AUTO_INCREMENT, ADD KEY (`id`);
# Max(id) is bigger than max(a) and max(b)
select max(a), max(b), max(id) from a;
# Mofidy ddl buffer size
set session innodb_ddl_buffer_size = 104857600;
alter table a drop column id, drop index id;
alter table a add column `id` bigint UNSIGNED NOT NULL AUTO_INCREMENT, ADD KEY (`id`);
# Max(id) is same with max(a) and max(b)
select max(a), max(b), max(id) from a;
drop table a;
drop PROCEDURE insert_into_tables;
set session innodb_ddl_buffer_size = @ori_val;
For the first select, the result is:
select max(a), max(b), max(id) from a;
max(a) max(b) max(id)
9999 9999 10000
For the second select, the result is:
select max(a), max(b), max(id) from a;
max(a) max(b) max(id)
9999 9999 9999
========================================
Okay, the above is the recurrence process, and we can know that this has nothing to do with the so-called innodb processing of auto-increment.
So what exactly causes this? Here is my analysis:
From the code of parallel DDL build, we can see:
1. Build row first: call ddl::Row::build
dberr_t Row::build(ddl::Context &ctx, dict_index_t *index, mem_heap_t *heap,
size_t type) noexcept {
...
auto &fts = ctx.m_fts;
if (fts.m_doc_id != nullptr && fts.m_doc_id->is_generated()) {
fts.m_doc_id->increment();
}
if (ctx.m_add_autoinc != ULINT_UNDEFINED) {
auto err = ctx.handle_autoinc(m_ptr);
if (err != DB_SUCCESS) {
return err;
}
}
return DB_SUCCESS;
}
Here, auto_inc will be handled.
2. After that, we will do `bulk_inserter`, the stack is:
bulk_inserter
-->add_row
---->bulk_add_row
`
3. In the function of `bulk_add_row`, if the key_buffer is full, `ddl::Cursor::copy_row` will be called, and `ddl::Row::build` will be called again, so autoinc is skipped!!!
[25 Sep 2024 11:58]
MySQL Verification Team
Hi Mr. Song, Can you please make a full and a complete test case, so that we can test it properly ???? Simply, make a full test case with all new additions ......... Thanks in advance ........
[25 Sep 2024 12:01]
Huaxiong Song
Please, I have provided a complete test case in my reply just now. . .
```
set @ori_val = select @@innodb_ddl_buffer_size;
create table a(a int, b int, primary key(a));
delimiter $$;
CREATE PROCEDURE insert_into_tables(IN num INTEGER)
BEGIN
declare x INT;
set x = 1;
while x < num do
INSERT INTO `a` (`a`, `b`) VALUES (x, x);
set x = x + 1;
end while;
end$$
delimiter ;$$
call insert_into_tables(10000);
alter table a add column `id` bigint UNSIGNED NOT NULL AUTO_INCREMENT, ADD KEY (`id`);
# Max(id) is bigger than max(a) and max(b)
select max(a), max(b), max(id) from a;
# Mofidy ddl buffer size
set session innodb_ddl_buffer_size = 104857600;
alter table a drop column id, drop index id;
alter table a add column `id` bigint UNSIGNED NOT NULL AUTO_INCREMENT, ADD KEY (`id`);
# Max(id) is same with max(a) and max(b)
select max(a), max(b), max(id) from a;
drop table a;
drop PROCEDURE insert_into_tables;
set session innodb_ddl_buffer_size = @ori_val;
```
[25 Sep 2024 12:03]
MySQL Verification Team
Thank you for your bug report. This is a verified bug now.
[26 Sep 2024 2:35]
Huaxiong Song
Tiny fix of bug 115136 (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.
Contribution: bug115136.patch (application/octet-stream, text), 3.66 KiB.
[26 Sep 2024 8:33]
MySQL Verification Team
Hi Mr. Song, Thank you for your contribution.
[23 Mar 8:03]
Huaxiong Song
Hello, I tested this again on 8.4.8 and found that the issue still exists, and my proposed fix is still effective. In some cases, this problem can become quite serious. As I mentioned earlier, the behavior is affected by the value of the `innodb_ddl_buffer_size` parameter. If the primary and replica use different values for this parameter, it can lead to data inconsistencies afterward. I believe this is an issue that should be fixed in a timely manner.
[24 Mar 21:46]
Jakub Lopuszanski
Hello Huaxiong Song, I feel like we owe you some update. The reason there was not much progress on this task recently, is that the issue is more complicated. You see, when we tried to apply your patch on 09 Feb 2025 (so more than a year ago), and ran your test, innodb_bug115136, the test, as written has passed. The test, though, only checks the sanity of maximum values. But, our developer took a look into the actual contents of all the rows, and discovered, that many of them have id=0. Indeed, if you modify your test like this: ``` set innodb_ddl_buffer_size = 65536; alter table a add column `id` bigint UNSIGNED NOT NULL AUTO_INCREMENT, ADD KEY (`id`); # Max(id) should be same with max(a) and max(b) select max(a), max(b), max(id) from a; select b from a where b not in (select id from a); +select * from a where id=0; ``` you will notice many rows are being assigned zero, which is definitely even worse, than having a "hole" in the numbering. Our developer investigated it, and concluded that this is because "row_build_w_add_vcol() resets the fts and auto inc values in the tuple". Alas, he's no longer with us. This task is now assigned to me, and I'll investigate the issue, as we need a more sophisticated fix.
[25 Mar 4:55]
Huaxiong Song
Hi Jakub, Thank you for testing the patch and reporting the `id=0` issue. You're right; the original fix was incomplete. Here is my analysis and the updated fix. ### Root Cause of the Original Bug (Skipped Values) During a parallel DDL table rebuild, each row from the old table goes through the following flow: 1. In the scan callback, `Row::build(ROW_COPY_POINTERS)` is called to construct the new row: - `row_build_w_add_vcol()` creates the tuple from the `add_cols` template - `handle_autoinc()` assigns the auto_increment value (e.g., 9533) to the `id` field - At this point, the field pointers in `row.m_ptr` point into the old table's B-tree page memory 2. The row is then passed to `bulk_add_row` → `add_to_key_buffer` to be added to the in-memory key buffer. 3. When the key buffer is full (controlled by `innodb_ddl_buffer_size`): - `cursor.copy_row()` is called, which invokes `Row::build(ROW_COPY_DATA)` to deep-copy the row data - `row_build_w_add_vcol()` recreates the tuple from the `add_cols` template again - The code then reaches `handle_autoinc()`, which increments the auto_increment sequence a second time - Result: this single row consumes two auto_increment values, but only the second one is used — the first value is skipped My original fix added an early return when `type == ROW_COPY_DATA`, skipping `handle_autoinc()` on the second call. This successfully avoided the double-increment (fixing the skipped values), but introduced a new problem. I have attached an updated patch that implements this fix. Could you please take another look? The core idea is to save the already-computed auto_increment value before the second `row_build_w_add_vcol()` call, and restore it afterward. Thanks again for the thorough review!
[25 Mar 4:55]
Huaxiong Song
Updated Fix (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.
Contribution: 0001-Fix-bug-115136.patch (application/octet-stream, text), 6.96 KiB.
[25 Mar 12:53]
Jakub Lopuszanski
I want to correct my earlier statement, to keep the records straight:
I've wrongly stated that the original testcase provided by you has passed on mysql-trunk with your older patch applied, which isn't true. Somehow I have missed, that there was a mismatch between expected result file and the actual test result, in that for the smaller buffer size we got a lot of missing values reported from:
```
select b from a where b not in (select id from a);
b
+99
+197
+295
+393
+491
+589
+687
...
+9997
```
I somehow ignored it, as I was focusing on the MAX's values, which were, as I've said, correct.
I think these missing values are caused by the interaction of your older patch with our fix for
Bug#37189985 crashing & widespread corruption of spatial indexes after INPLACE alter table
which was applied in 5.7.49, 8.0.41, 8.4.4, 9.2.0. (BTW: you claimed your patch works on 8.4.8 which is one lose end here)
The fix for Bug#37189985 has modified `Context::handle_autoinc` to create a deep copy of the auto inc field m_ptr->fields[ctx.m_add_autoinc].data, so that it is no longer shared with m_add_cols->fields[ctx.m_add_autoinc].data.
The way it is relevant to your older patch is that there were two calls to `Row::build` for row with b=99, first with type==ROW_COPY_POINTERS, and later with type==ROW_COPY_DATA, and the
```
m_ptr = row_build_w_add_vcol(type, index, m_rec, m_offsets, ctx.m_new_table,
m_add_cols, ctx.m_add_v, ctx.m_col_map, &m_ext,
heap);
```
constructs m_ptr using shallow copy of fields of m_add_cols.
(To be more specific: m_ptr is a tuple which mixes fields referencing the m_rec (or its copy in case of ROW_COPY_DATA), and the newly added columns, like our autoinc are taken from m_add_cols instead, by shallow copy).
So, without the fix for Bug#37189985, the first call with type==ROW_COPY_POINTERS, when executing Context::handle_autoinc was putting the autoincrement value generated from m_sequence in m_ptr's field shared with m_add_cols, which in the second call with type==ROW_COPY_DATA was used to again create m_ptr. This way the m_ptr->fields[ctx.m_add_autoinc] in the second call had the value from m_sequence, not zero.
Indeed, if I revert the fix for Bug#37189985 like this:
```
dberr_t Context::handle_autoinc(const dtuple_t *dtuple,
mem_heap_t *heap) noexcept {
ut_ad(m_add_autoinc != ULINT_UNDEFINED);
ut_ad(m_add_autoinc < m_new_table->get_n_user_cols());
const auto dfield = dtuple_get_nth_field(dtuple, m_add_autoinc);
/* Perform a deep copy of the field because for spatial indexes,
the default tuple allocation is overwritten, as tuples are
processed at the end of the page. */
- dfield_dup(dfield, heap);
+ (void)heap;
```
then your test passes fine on mysql-trunk with your older patch.
However, when the fix for Bug#37189985 is present, then the call to Context::handle_autoinc puts the value from m_sequence in the m_ptr->field[ctx.m_add_autoinc].data which is a new buffer independent of m_add_cols. This means that the m_add_cols->field[ctx.m_add_autoinc].data retains its original value, which seems to be all zeros. This in turn means that the subsequent call with type==ROW_COPY_DATA initializes m_ptr from m_add_cols which still has zeros in it, which results in creating a row with id=0.
Your newer patch seems to work around this issue, by stashing the m_ptr->field[ctx.m_add_autoinc] during the second call before calling row_build_w_add_vcol, so that it can then be patched back afterwards. This seems to be based on assumption that the m_ptr->field[ctx.m_add_autoinc] at the start of the second call to Row::build(..) is the one we have generated near the end of the first call to Row::build(..) when doing ctx.handle_autoinc(m_ptr, heap).
Under this assumption, the stashed value is the one computed correctly, which we should use now.
Thank you for this new patch, looks very promising!
What worries me, though, is that it is far from clear to me (ATM) that we can assume that during the call with type==ROW_COPY_DATA the m_ptr contains whatever was left there during last call with type==ROW_COPY_POINTERS - i.e. that nothing interfered/modified anything important here.
In particular, I wonder: if that assumption was indeed true, then why do we even bother to call row_build_w_add_vcol instead of "simply" reusing the already generated m_ptr and just make it's fields "independent" by deep cloning them, say by using m_ptr->deep_copy(mem_heap_t *heap)?
I think next step to accept your latest patch would be to somehow prove that:
1. indeed there's no chance m_ptr gets modified in-between the two calls
2. that it is ok to make it part of the contract for Row::build, that one can call it with type==ROW_COPY_DATA only "immediately after" calling it with type==ROW_COPY_POINTERS
3. that the way you stash the field (by simply using default = operator) is correct. I think most places in code use `dfield_copy` for some reason, which under the hood does the same thing, but perhaps the idea here was to have a single point in code where we can asserts? Anyway, this seems to be a "shallow" copy in that the .data points to a fragment of memory which we will soon "unreference" when row_build_w_add_vcol will rebuild m_ptr. I think this is also safe for the reason that all of this sits in `heap` which we will not free anytime soon. Still, I need to verify the lifetimes here
4. that the way you restore the field by dfield_copy + dfield_dup is the right thing to do. Probably is.
I think I have some handwaving argument for 1 & 2 which goes like this:
The "second call", i.e. row.build(m_ctx, index(), heap, ROW_COPY_DATA) happens from `err = cursor.copy_row(thread_id, row)` when handling DB_OVERFLOW returned by `add_to_key_buffer(ctx, mv_rows_added)` - which was a call which tried to add the row to the keysort buffer via copy_row(ctx, mv_rows_added) -> copy_columns(ctx, mv_rows_added, write_doc_id) which if you look inside operates on ctx.m_row.m_ptr.
So indeed, it looks like the row for which we are calling ROW_COPY_DATA has the ctx.m_row.m_ptr still pointing to the row we have just prepared in the call with ROW_COPY_POINTERS.
In other words the flow which I see in debugger is:
```
auto err = row.build(m_ctx, m_index, heap, ROW_COPY_POINTERS);
..
err = bulk_inserter(read_ctx->thread_ctx(), row);
-> const auto err = builder->add_row(*this, row, thread_id, [&]() {...})
-> err = bulk_add_row(cursor, row, thread_id, std::move..)
-> err = cursor.copy_row(thread_id, row);
-> return row.build(m_ctx, index(), heap, ROW_COPY_DATA);
```
AFAICT this is the only call to row.build(...type=ROW_COPY_DATA) in the codebase, so I think this is fine.
Separately, I wonder if this is indeed the cleanest way to solve the problem here.
You are definitely right that calling m_sequence++ twice, is bad.
But, I wonder, if we could perhaps add `bool reuse=false` parameter to `Context::handle_autoinc(..)` and pass true in case of ROW_COPY_DATE to indicate we don't want this increment to happen?
Or, maybe, as I've suggested above, this whole ROW_COPY_POINTERS mode is over-complicated and should simply do
```
if (type == ROW_COPY_DATA) {
for (size_t i = 0; i < m_ptr->n_fields; ++i) {
dfield_dup(&m_ptr->fields[i], heap);
}
return DB_SUCCESS;
}
```
instead?
In particular, I worry, if we have a similar problem with FTS here:
```
auto &fts = ctx.m_fts;
if (fts.m_doc_id != nullptr && fts.m_doc_id->is_generated()) {
fts.m_doc_id->increment();
}
```
which perhaps would be easier to fix with
Let me know what you think.
[27 Mar 9:04]
Huaxiong Song
Hi Jakub,
Thank you very much for the thorough and detailed analysis.
I'd like to address your two verification concerns, and then propose adopting your suggested Approach B.
I want to clarify the precise root cause, as my initial analysis was not accurate enough. I originally described it as "row_build_waddvcol resets the auto_increment value," which is correct but imprecise. I had not initially noticed the interaction with the Bug#37189985 fix.The actual mechanism is as follows. Before Bug#37189985, m_ptr's auto_increment field and m_add_cols's auto_increment field shared the same underlying memory (ptr_A) via a shallow dtuple_copy in row_build_low. So when handle_autoinc wrote the auto_increment value into m_ptr, it also implicitly updated m_add_cols. A subsequent row_build_w_add_vcol call would then dtuple_copy(add_cols, heap) and pick up the correct value from m_add_cols.Bug#37189985 added dfield_dup(dfield, heap) in handle_autoinc (ddl0ctx.cc), which deep-copies the field data into a new buffer (ptr_B) before writing the auto_increment value. This breaks the memory sharing: the auto_increment value is written to ptr_B (now owned by m_ptr), while m_add_cols still points to the original ptr_A which remains zero. When the second row_build_w_add_vcol(ROW_COPY_DATA) call does dtuple_copy(add_cols, heap), it creates the tuple from m_add_cols and naturally gets zero for the auto_increment field.This is why Approach B — skipping row_build_w_add_vcol entirely for ROW_COPY_DATA and just deep-copying the existing m_ptr fields — avoids the problem at its root.
Regarding Concern 1: Is `m_ptr` unmodified between the two `Row::build` calls?
Yes. After `Row::build(ROW_COPY_POINTERS)` returns, `m_ptr` flows through the following path before `Row::build(ROW_COPY_DATA)` is called:
1. `Builder::bulk_add_row` → `Builder::add_to_key_buffer` → `Builder::copy_row` → `Builder::copy_columns`:
- `copy_columns` reads from `m_ptr` via `dtuple_get_nth_field(ctx.m_row.m_ptr, col_no)` (ddl0builder.cc:918), then does `dfield_copy(field, src_field)` (ddl0builder.cc:921) — this is a shallow copy FROM `m_ptr` to the key_buffer's fields. It never writes back to `m_ptr`.
2. `Key_sort_buffer::deep_copy`:
- This calls `dfield_dup` on the key_buffer's own fields (ddl0buffer.cc:86-94), not on `m_ptr`'s fields.
Neither step modifies any `dfield_t` within `m_ptr`. The only modification to `m_ptr` fields happens in `handle_autoinc` during the `ROW_COPY_POINTERS` path (before `copy_columns` is called), where `dfield_dup` deep-copies the auto_increment field's data into the scan heap — but this doesn't affect the field's semantic value, only its storage location.
Regarding Concern 2: Is `ROW_COPY_DATA` always preceded by `ROW_COPY_POINTERS`?
Yes. `Parallel_cursor::copy_row` (ddl0par-scan.cc:404-413) is the only call site for `Row::build(ROW_COPY_DATA)` in the entire codebase. It is called from `Builder::bulk_add_row` (ddl0builder.cc:1477):
```
cursor.copy_row(thread_id, row) // ROW_COPY_DATA
```
This is only reached when the key_buffer overflows (`DB_OVERFLOW` from `add_to_key_buffer`), which means the scan callback has already called `Row::build(ROW_COPY_POINTERS)` for this row (ddl0par-scan.cc:362) and the row has already gone through `handle_autoinc`. The purpose of `copy_row` is to deep-copy the row data before releasing the page latch (for `log_free_check` during `mtr.commit`), so the row can be re-added to the cleared key_buffer after `insert_direct`.
Adopting Approach B: Deep-copy `m_ptr` fields instead of re-calling `row_build_w_add_vcol`
I agree that Approach B is cleaner and avoids the root cause entirely. Instead of calling `row_build_w_add_vcol(ROW_COPY_DATA, ...)` (which rebuilds the tuple from the `m_add_cols` template and resets all newly added column values), we can simply deep-copy all existing `m_ptr` fields in place:
```cpp
dberr_t Row::build(ddl::Context &ctx, dict_index_t *index, mem_heap_t *heap,
size_t type) noexcept {
ut_ad(rec_offs_any_null_extern(index, m_rec, m_offsets) == nullptr);
if (type == ROW_COPY_DATA) {
ut_ad(m_ptr != nullptr);
for (ulint i = 0; i < dtuple_get_n_fields(m_ptr); ++i) {
dfield_dup(dtuple_get_nth_field(m_ptr, i), heap);
}
for (ulint i = 0; i < dtuple_get_n_v_fields(m_ptr); ++i) {
dfield_dup(dtuple_get_nth_v_field(m_ptr, i), heap);
}
return DB_SUCCESS;
}
m_ptr = row_build_w_add_vcol(type, index, m_rec, m_offsets, ctx.m_new_table,
m_add_cols, ctx.m_add_v, ctx.m_col_map, &m_ext,
heap);
// ...
}
```
This approach has several advantages:
- Preserves all computed values naturally: auto_increment, FTS doc_id, and any other field values set during the `ROW_COPY_POINTERS` path are all retained without needing individual save/restore logic.
- Avoids the root cause entirely: we never re-call `row_build_w_add_vcol` for `ROW_COPY_DATA`, so the `dtuple_copy(add_cols, heap)` reset inside `row_build_low` is never triggered.
- `m_ext` remains valid: `m_ext` was created during `ROW_COPY_POINTERS` in the scan heap. The scan heap is only cleared via `mem_heap_empty` in the `finish_callback` between pages, not within the same page callback. Since `copy_row` is called before `latch_release` and the scan callback hasn't returned yet, `m_ext->buf` (allocated in the scan heap) is still valid when the row is subsequently re-processed by `copy_columns`.
- External (BLOB) fields are safe: For ext fields in `m_ptr`, their data (inline portion + 20-byte LOB reference) is deep-copied into the heap by `dfield_dup`. The actual BLOB content is not needed at this stage — it will be fetched later from the original table's LOB pages via `copy_blobs` → `lob::btr_copy_externally_stored_field_func` during the `insert_direct` path.
Regarding FTS doc_id:Approach B is also safe for FTS doc_id. The doc_id counter is maintained in the context object (ctx.m_fts.m_doc_id), not in the tuple itself. During the ROW_COPY_POINTERS path, fts.m_doc_id->increment() advances the counter, and the actual doc_id value is written into the tuple field later by fts_add_doc_id() inside copy_columns (ddl0builder.cc:950). With Approach B, when ROW_COPY_DATA is called, we return early after deep-copying all m_ptr fields — this correctly preserves the doc_id field value that was already written during copy_columns, and avoids a redundant increment() call which would cause doc_id gaps. So no special handling is needed for FTS doc_id.
I will prepare an updated patch based on Approach B. If there are any other details or concerns, I'd be happy to discuss further.
[27 Mar 10:35]
Huaxiong Song
Updated Fix 0327 (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.
Contribution: 0001-Fix-bug-115136.patch (application/octet-stream, text), 5.98 KiB.

Description: When add an auto_increment column, some records are skipped. How to repeat: Run mtr with the test-case: # Test case: create table a(a int, b int, primary key(a)); delimiter $$; CREATE PROCEDURE insert_into_tables(IN num INTEGER) BEGIN declare x INT; set x = 1; while x < num do INSERT INTO `a` (`a`, `b`) VALUES (x, x); set x = x + 1; end while; end$$ delimiter ;$$ call insert_into_tables(10000); alter table a add column `id` bigint UNSIGNED NOT NULL AUTO_INCREMENT, ADD KEY (`id`); select max(a), max(b), max(id) from a; drop table a; drop PROCEDURE insert_into_tables; #==== In my test, the result is: select max(a), max(b), max(id) from a; max(a) max(b) max(id) 9999 9999 10000 I think max(id) should be equal to 9999. And I try to run test case with different values of innodb_autoinc_lock_mode(0, 1, 2), there is no change occurred.