• Guilhem Bichot's avatar
    Fix for BUG#31612 · 84c1fffa
    Guilhem Bichot authored
    "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
    84c1fffa
log.cc 156 KB