Commit f8670e2c authored by Guilhem Bichot's avatar Guilhem Bichot

Fix for BUG#31612

"Trigger fired multiple times leads to gaps in auto_increment sequence".
The bug was that if a trigger fired multiple times inside a top
statement (for example top-statement is a multi-row INSERT,
and trigger is ON INSERT), and that trigger inserted into an auto_increment
column, then gaps could be observed in the auto_increment sequence,
even if there were no other users of the database (no concurrency).
It was wrong usage of THD::auto_inc_intervals_in_cur_stmt_for_binlog.
Note that the fix changes "class handler", I'll tell the Storage Engine API team.
parent 0b097b77
......@@ -161,3 +161,30 @@ SELECT @a, @b;
1 1
DROP TABLE t2, t1;
End of 5.0 tests
BUG#31612
Trigger fired multiple times leads to gaps in auto_increment sequence
create table t1 (a int, val char(1)) engine=InnoDB;
create table t2 (b int auto_increment primary key,
val char(1)) engine=InnoDB;
create trigger t1_after_insert after
insert on t1 for each row insert into t2 set val=NEW.val;
insert into t1 values ( 123, 'a'), ( 123, 'b'), ( 123, 'c'),
(123, 'd'), (123, 'e'), (123, 'f'), (123, 'g');
insert into t1 values ( 654, 'a'), ( 654, 'b'), ( 654, 'c'),
(654, 'd'), (654, 'e'), (654, 'f'), (654, 'g');
select * from t2 order by b;
b val
1 a
2 b
3 c
4 d
5 e
6 f
7 g
8 a
9 b
10 c
11 d
12 e
13 f
14 g
......@@ -162,3 +162,16 @@ DROP TABLE t2, t1;
--echo End of 5.0 tests
--echo BUG#31612
--echo Trigger fired multiple times leads to gaps in auto_increment sequence
create table t1 (a int, val char(1)) engine=InnoDB;
create table t2 (b int auto_increment primary key,
val char(1)) engine=InnoDB;
create trigger t1_after_insert after
insert on t1 for each row insert into t2 set val=NEW.val;
insert into t1 values ( 123, 'a'), ( 123, 'b'), ( 123, 'c'),
(123, 'd'), (123, 'e'), (123, 'f'), (123, 'g');
insert into t1 values ( 654, 'a'), ( 654, 'b'), ( 654, 'c'),
(654, 'd'), (654, 'e'), (654, 'f'), (654, 'g');
select * from t2 order by b;
......@@ -2165,7 +2165,12 @@ prev_insert_id(ulonglong nr, struct system_variables *variables)
- In both cases, the reserved intervals are remembered in
thd->auto_inc_intervals_in_cur_stmt_for_binlog if statement-based
binlogging; the last reserved interval is remembered in
auto_inc_interval_for_cur_row.
auto_inc_interval_for_cur_row. The number of reserved intervals is
remembered in auto_inc_intervals_count. It differs from the number of
elements in thd->auto_inc_intervals_in_cur_stmt_for_binlog() because the
latter list is cumulative over all statements forming one binlog event
(when stored functions and triggers are used), and collapses two
contiguous intervals in one (see its append() method).
The idea is that generated auto_increment values are predictable and
independent of the column values in the table. This is needed to be
......@@ -2249,8 +2254,6 @@ int handler::update_auto_increment()
handler::estimation_rows_to_insert was set by
handler::ha_start_bulk_insert(); if 0 it means "unknown".
*/
uint nb_already_reserved_intervals=
thd->auto_inc_intervals_in_cur_stmt_for_binlog.nb_elements();
ulonglong nb_desired_values;
/*
If an estimation was given to the engine:
......@@ -2262,17 +2265,17 @@ int handler::update_auto_increment()
start, starting from AUTO_INC_DEFAULT_NB_ROWS.
Don't go beyond a max to not reserve "way too much" (because
reservation means potentially losing unused values).
Note that in prelocked mode no estimation is given.
*/
if (nb_already_reserved_intervals == 0 &&
(estimation_rows_to_insert > 0))
if ((auto_inc_intervals_count == 0) && (estimation_rows_to_insert > 0))
nb_desired_values= estimation_rows_to_insert;
else /* go with the increasing defaults */
{
/* avoid overflow in formula, with this if() */
if (nb_already_reserved_intervals <= AUTO_INC_DEFAULT_NB_MAX_BITS)
if (auto_inc_intervals_count <= AUTO_INC_DEFAULT_NB_MAX_BITS)
{
nb_desired_values= AUTO_INC_DEFAULT_NB_ROWS *
(1 << nb_already_reserved_intervals);
(1 << auto_inc_intervals_count);
set_if_smaller(nb_desired_values, AUTO_INC_DEFAULT_NB_MAX);
}
else
......@@ -2340,8 +2343,9 @@ int handler::update_auto_increment()
{
auto_inc_interval_for_cur_row.replace(nr, nb_reserved_values,
variables->auto_increment_increment);
auto_inc_intervals_count++;
/* Row-based replication does not need to store intervals in binlog */
if (!thd->current_stmt_binlog_row_based)
if (mysql_bin_log.is_open() && !thd->current_stmt_binlog_row_based)
thd->auto_inc_intervals_in_cur_stmt_for_binlog.append(auto_inc_interval_for_cur_row.minimum(),
auto_inc_interval_for_cur_row.values(),
variables->auto_increment_increment);
......@@ -2461,6 +2465,7 @@ void handler::ha_release_auto_increment()
release_auto_increment();
insert_id_for_cur_row= 0;
auto_inc_interval_for_cur_row.replace(0, 0, 0);
auto_inc_intervals_count= 0;
if (next_insert_id > 0)
{
next_insert_id= 0;
......
......@@ -1129,6 +1129,13 @@ class handler :public Sql_alloc
inserter.
*/
Discrete_interval auto_inc_interval_for_cur_row;
/**
Number of reserved auto-increment intervals. Serves as a heuristic
when we have no estimation of how many records the statement will insert:
the more intervals we have reserved, the bigger the next one. Reset in
handler::ha_release_auto_increment().
*/
uint auto_inc_intervals_count;
handler(handlerton *ht_arg, TABLE_SHARE *share_arg)
:table_share(share_arg), table(0),
......@@ -1137,7 +1144,8 @@ class handler :public Sql_alloc
ref_length(sizeof(my_off_t)),
ft_handler(0), inited(NONE),
locked(FALSE), implicit_emptied(0),
pushed_cond(0), next_insert_id(0), insert_id_for_cur_row(0)
pushed_cond(0), next_insert_id(0), insert_id_for_cur_row(0),
auto_inc_intervals_count(0)
{}
virtual ~handler(void)
{
......
......@@ -4011,11 +4011,6 @@ bool MYSQL_BIN_LOG::write(Log_event *event_info)
DBUG_PRINT("info",("number of auto_inc intervals: %u",
thd->auto_inc_intervals_in_cur_stmt_for_binlog.
nb_elements()));
/*
If the auto_increment was second in a table's index (possible with
MyISAM or BDB) (table->next_number_keypart != 0), such event is
in fact not necessary. We could avoid logging it.
*/
Intvar_log_event e(thd, (uchar) INSERT_ID_EVENT,
thd->auto_inc_intervals_in_cur_stmt_for_binlog.
minimum());
......
......@@ -1524,6 +1524,9 @@ class THD :public Statement,
then the latter INSERT will insert no rows
(first_successful_insert_id_in_cur_stmt == 0), but storing "INSERT_ID=3"
in the binlog is still needed; the list's minimum will contain 3.
This variable is cumulative: if several statements are written to binlog
as one (stored functions or triggers are used) this list is the
concatenation of all intervals reserved by all statements.
*/
Discrete_intervals_list auto_inc_intervals_in_cur_stmt_for_binlog;
/* Used by replication and SET INSERT_ID */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment