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.

mysql-test/r/trigger-trans.result:
  result; before the bugfix, the sequence was 1,2,4,6,8,10,12...
mysql-test/t/trigger-trans.test:
  test for BUG#31612
sql/handler.cc:
  See revision comment of handler.h.
  As THD::auto_inc_intervals_in_cur_stmt_for_binlog is cumulative
  over all trigger invokations by the top statement, the
  second invokation of the trigger arrived in handler::update_auto_increment()
  with already one interval in
  THD::auto_inc_intervals_in_cur_stmt_for_binlog. The method thus
  believed it had already reserved one interval for that invokation,
  thus reserved a twice larger interval (heuristic when we don't know
  how large the interval should be: we grow by powers of two). InnoDB
  thus increased its internal per-table auto_increment counter by 2
  while only one row was to be inserted. Hence a gap in the sequence.
  The fix is to use the new handler::auto_inc_intervals_count.
  Note that the trigger's statement knows how many rows it is going
  to insert, but provides estimation_rows_to_insert == 0 (see comments
  in sql_insert.cc why triggers don't call handler::ha_start_bulk_insert()).
  * removing white space at end of line
  * we don't need to maintain THD::auto_inc_intervals_in_cur_stmt_for_binlog
  if no binlogging or if row-based binlogging. Using auto_inc_intervals_count in
  the heuristic makes the heuristic independent of binary logging, which is good.
sql/handler.h:
  THD::auto_inc_intervals_in_cur_stmt_for_binlog served
   - for binlogging
   - as a heuristic when we have no estimation of how many records the
     statement will insert.
  But the first goal needs to be cumulative over all statements which
  form a binlog event, while the second one needs to be attached to each
  statement. THD::auto_inc_intervals_in_cur_stmt_for_binlog is cumulative,
  leading to BUG#31612. So we introduce handler::auto_inc_intervals_count
  for the second goal. See the revision comment of handler.cc.
  A smaller issue was that, even when the binlog event was only one
  statement (no triggers, no stored functions),
  THD::auto_inc_intervals_in_cur_stmt.nb_elements() could be lower than
  the number of reserved intervals (fooling the heuristic), because its
  append() method collapses two contiguous intervals in one.
  Note that as auto_inc_intervals_count is in class 'handler' and not
  in class 'THD', it does not need to be handled in
  THD::reset|restore_sub_statement_state().
sql/log.cc:
  Comment is wrong: if auto_increment is second, in handler::update_auto_increment()
  'append' is false and so auto_inc_intervals_in_cur_stmt_for_binlog
  is empty, we do not come here.
sql/sql_class.h:
  comment

1 files changed, 9 insertions, 1 deletions

diff --git a/sql/handler.h b/sql/handler.h
index 3c5db8a0dff..b943e188962 100644
--- a/sql/handler.h
+++ b/sql/handler.h
@@ -1129,6 +1129,13 @@ public:
     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 @@ public:
     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)
   {