Hello Guilhem!
Here are my comments about your patch:
* Guilhem Bichot <guilhem@stripped> [07/10/22 23:01]:
> ChangeSet@stripped, 2007-10-22 20:52:05+02:00, guilhem@stripped +8 -0
> Fix for BUG#31612
> "Trigger fired multiple times leads to gaps in auto_increment sequence".
> Bug was never experienced by users because it occurs only with the fix
> for BUG31540 "incorrect auto_increment values used for multi-row insert
> trigger" which is not yet in the main 5.1.
> 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.
...
> diff -Nrup a/sql/handler.h b/sql/handler.h
> --- a/sql/handler.h 2007-09-07 15:41:46 +02:00
> +++ b/sql/handler.h 2007-10-22 20:52:00 +02:00
> @@ -1050,6 +1050,13 @@ public:
> inserter.
> */
> Discrete_interval auto_inc_interval_for_cur_row;
> + /**
> + @brief 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().
> + */
> + uint32 auto_inc_intervals_count;
AFAIU by adding new member to "handler" class you are changing our SE API
and potentially create problems for pluggable storage engines. IMO it
makes sense at least to notify our SE API team about your changes. Please
write a letter describing them to Sanjay Manwani <Sanjay.Manwani@stripped>
(actually it might be a good idea to ask him about appropriate list/
procedure for such notifications).
Also could you please elaborate why do you use uint32 here and not uint ?
Old code used Discrete_intervals_list::elements, which is uint.
>
> handler(handlerton *ht_arg, TABLE_SHARE *share_arg)
> :table_share(share_arg), table(0),
> @@ -1058,7 +1065,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)
> {
I think it is OK to push this patch after considering/addressing
above points.
--
Dmitry Lenev, Software Developer
MySQL AB, www.mysql.com
Are you MySQL certified? http://www.mysql.com/certification
