Re: bk commit into 5.1 tree (hezx:1.2534) BUG#33029

[He Zhenxing]

Hi Andrei

See comments at the end.

> Hi Andrei
> 
> Thank you!
> 
> On 2008-03-11 Tue 17:39 +0200，Andrei Elkin wrote：
> > Zhen Xing, hello.
> > 
> > The patch for 5.0 addresses its half of the total problem.
> > The idea - 
> > 
> > @@ -622,6 +622,7 @@ void THD::cleanup_after_query()
> > +    insert_id_used= 0;
> > 
> > must be correct.
> > 
> > In the following I am leaving my comments along the 5.1 patch to mean
> > in some places a remark belongs to both csets.
> > 
> > 
> > > Below is the list of changes that have just been committed into a local
> > > 5.1 repository of hezx.  When hezx does a push these changes
> > > will be propagated to the main repository and, within 24 hours after the
> > > push, to the public repository.
> > > For information on how to access the public repository
> > > see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
> > >
> > > ChangeSet@stripped, 2008-03-11 13:25:26+08:00, hezx@stripped +11 -0
> > >   BUG#33029 5.0 to 5.1 replication fails on dup key when inserting
> > >   using a trig in SP
> > >   
> > >   Prior to MySQL 5.1.12 (including 5.0.x), when a stored routine or
> > 
> > I'd put it this way: 
> > 
> >     For all 5.0 and up to 5.1.12 exclusive  \footnote{i judge basing on the
> >     following references in the test, please correct if wrong}
> >     
> >     when a stored function  or trigger ...
> > 
> 
> good
> 
> > Would be nice to refer to a fix or wl that made 5.1.12 so advanced.
> > 
> > >   trigger caused an INSERT into an AUTO_INCREMENT column, the
> > >   generated AUTO_INCREMENT value should not be written into the binary
> > >   log, which means if a statement does not generate AUTO_INCREMENT
> > 
> > >   valule itself,
> > 
> > "itself" would be better be said as `by the top level statement' to
> > contrast to a sf() or a trigger() that comprise a substatement rooted
> > from the top level.
> > 
> 
> ok
> 
> > >   there will be no Intvar event (SET INSERT_ID)
> > >   associated with it even if one of the stored routine or trigger
> > >   caused generation of such a value. And meanwhile, when executing a
> > >   stored routine or trigger, it would ignore the INSERT_ID value even
> > >   if there is a INSERT_ID value available set by a SET INSERT_ID
> > >   statement.
> > >   
> > 
> > 
> > >   After MySQL 5.1.12, the generated AUTO_INCREMENT value is written
> > 
> > I'd put it 
> > 
> >     Starting from 5.1.12, ...
> > 
> 
> nice!
> 
> > >   into the binary log, and the value will be used if availabe when
> > >   executing the stored routine or trigger.
> > >   
> > >   Prior fix of this bug in MySQL 5.0 and prior MySQL 5.1.12
> > >   (referenced as the buggy versions in the text below), when one
> > >   statement that generates AUTO_INCREMENT value by itself was executed
> > >   in the body of a SP, all statements in the SP after this statement
> > >   would be treated as if they had generated AUTO_INCREMENT by itself.
> > >   When a statement that did not generate AUTO_INCREMENT value by
> > >   itself but by a function/trigger called by it, an erroenous Intvar
> > >   event would be associated with the statement, this erroenous
> > >   INSERT_ID value wouldn't cause problem when replicating between
> > >   masters and slaves of 5.0.x or prior 5.1.12, because the erroneous
> > >   INSERT_ID value was not used when executing functions/triggers. But
> > >   when replicating from buggy versions to 5.1.12 or newer, which will
> > >   use the INSERT_ID value in functions/triggers, the erronous value
> > >   will be used.
> > 
> > Please add here - that the errornous value caused the observed duplicate
> > error in the bug description.
> 
> right
> 
> > >   
> > >   The patch for 5.1 fixed it to ignore the SET INSERT_ID value when
> > >   executing functions/triggers if it is replicating from a master
> > >   of buggy versions, another patch for 5.0 fixed it not to generate
> > >   the erroneous Intvar event.
> > 
> > Okay.
> > 
> > Please make sure, I'd put a todo item in these comments, to document
> > as `CHANGES FOR DOCUMENTATION PROPOSAL' the 5.0 limitation applies to
> > 5.1.x where x < 12.
> 
> I have already reported a bug to request the change, and Jon has already
> updated the document. Please check:
> 
> http://dev.mysql.com/doc/refman/5.1/en/replication-features-autoincid.html
> 
> 
> > 
> > >
> > >   mysql-test/include/show_binlog_events.inc@stripped, 2008-03-11 13:25:22+08:00,
> hezx@stripped +8 -3
> > >     add $_binlog_start parameter to show binlog events from a given
> position
> > >
> > >   mysql-test/std_data/bug33029-slave-relay-bin.000001@stripped, 2008-03-11
> 13:25:23+08:00, hezx@stripped +89 -0
> > >     relay logs from a buggy 5.0 master for test case of BUG#33029
> > >
> > >   mysql-test/std_data/bug33029-slave-relay-bin.000001@stripped, 2008-03-11
> 13:25:23+08:00, hezx@stripped +0 -0
> > >
> > >   mysql-test/suite/binlog/r/binlog_auto_increment_bug33029.result@stripped,
> 2008-03-11 13:25:23+08:00, hezx@stripped +33 -0
> > >     Test if the slave can process relay logs from a buggy master of
> BUG#33029
> > >
> > >   mysql-test/suite/binlog/r/binlog_auto_increment_bug33029.result@stripped,
> 2008-03-11 13:25:23+08:00, hezx@stripped +0 -0
> > >
> > >   mysql-test/suite/binlog/t/binlog_auto_increment_bug33029-master.opt@stripped,
> 2008-03-11 13:25:23+08:00, hezx@stripped +1 -0
> > >     Test if the slave can process relay logs from a buggy master of
> BUG#33029
> > >
> > >   mysql-test/suite/binlog/t/binlog_auto_increment_bug33029-master.opt@stripped,
> 2008-03-11 13:25:23+08:00, hezx@stripped +0 -0
> > >
> > >   mysql-test/suite/binlog/t/binlog_auto_increment_bug33029.test@stripped,
> 2008-03-11 13:25:23+08:00, hezx@stripped +24 -0
> > >     Test if the slave can process relay logs from a buggy master of
> BUG#33029
> > >
> > >   mysql-test/suite/binlog/t/binlog_auto_increment_bug33029.test@stripped,
> 2008-03-11 13:25:23+08:00, hezx@stripped +0 -0
> > >
> > >   mysql-test/suite/rpl/r/rpl_auto_increment_bug33029.result@stripped, 2008-03-11
> 13:25:23+08:00, hezx@stripped +168 -0
> > >     Add test for BUG#33029
> > >
> > >   mysql-test/suite/rpl/r/rpl_auto_increment_bug33029.result@stripped, 2008-03-11
> 13:25:23+08:00, hezx@stripped +0 -0
> > >
> > >   sql/slave.cc@stripped, 2008-03-11 13:25:22+08:00, hezx@stripped +22 -0
> > >     Add function to check for bug#33029
> > >
> > >   sql/slave.h@stripped, 2008-03-11 13:25:22+08:00, hezx@stripped +1 -0
> > >     Add function to check for bug#33029
> > >
> > >   sql/sql_class.cc@stripped, 2008-03-11 13:25:22+08:00, hezx@stripped +11
> -0
> > >     if master has bug#33029, reset auto_inc_intervals_forced for sub
> statements
> > >
> > >   sql/sql_class.h@stripped, 2008-03-11 13:25:23+08:00, hezx@stripped +1
> -0
> > >     Add member to save and restore auto_inc_intervals_forced
> > >
> > >   sql/structs.h@stripped, 2008-03-11 13:25:23+08:00, hezx@stripped +22 -0
> > >     Add functions to save and restore Discrete_intervals_list object
> > >
> > 
> > that's a good idea in below!
> > 
> > > diff -Nrup a/mysql-test/include/show_binlog_events.inc
> b/mysql-test/include/show_binlog_events.inc
> > > --- a/mysql-test/include/show_binlog_events.inc	2008-01-29 21:43:03 +08:00
> > > +++ b/mysql-test/include/show_binlog_events.inc	2008-03-11 13:25:22 +08:00
> > > @@ -1,5 +1,10 @@
> > 
> > > ---let $binlog_start=106
> > still, i'd leave the constant
> > 
> >    let $binlog_start=106;
> > 
> > to use in the following assignement.
> > In future, we may consider to have a list of mysql-test global
> variables/constants that are
> > sensitive to mysql version and $binlog_start would one of them.
> > 
> 
> Good that $binlog_start will become a pre-defined global variable, I'll
> change it back.
> 
> > 
> > 
> > > ---replace_result $binlog_start <binlog_start>
> > > +# $_binlog_start can be set by caller or take a default value
> > > +
> > > +if (!$_binlog_start)
> > > +{
> > > +  let $_binlog_start=106;
> > > +}
> > > +--replace_result $_binlog_start <binlog_start>
> > >  --replace_column 2 # 4 # 5 #
> > >  --replace_regex /\/\* xid=.* \*\//\/* XID *\// /table_id: [0-9]+/table_id:
> #/ /file_id=[0-9]+/file_id=#/
> > > ---eval show binlog events from $binlog_start
> > > +--eval show binlog events from $_binlog_start
> > > Binary files a/mysql-test/std_data/bug33029-slave-relay-bin.000001 and
> b/mysql-test/std_data/bug33029-slave-relay-bin.000001 differ
> > > diff -Nrup
> a/mysql-test/suite/binlog/r/binlog_auto_increment_bug33029.result
> b/mysql-test/suite/binlog/r/binlog_auto_increment_bug33029.result
> > > --- /dev/null	Wed Dec 31 16:00:00 196900
> > > +++
> b/mysql-test/suite/binlog/r/binlog_auto_increment_bug33029.result	2008-03-11 13:25:23
> +08:00
> > > @@ -0,0 +1,33 @@
> > > +change master to
> > > +MASTER_HOST='dummy.localdomain',
> > > +RELAY_LOG_FILE='slave-relay-bin.000001',
> > > +RELAY_LOG_POS=4;
> > > +start slave sql_thread;
> > > +select MASTER_POS_WAIT('master-bin.000001', 3776);
> > > +# Result on slave
> > > +SELECT * FROM t1;
> > > +id
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +11
> > > +SELECT * FROM t2;
> > > +id
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +11
> > > +12
> > > +13
> > > +14
> > > +15
> > > +16
> > > +17
> > > +18
> > > +19
> > > diff -Nrup
> a/mysql-test/suite/binlog/t/binlog_auto_increment_bug33029-master.opt
> b/mysql-test/suite/binlog/t/binlog_auto_increment_bug33029-master.opt
> > > --- /dev/null	Wed Dec 31 16:00:00 196900
> > > +++
> b/mysql-test/suite/binlog/t/binlog_auto_increment_bug33029-master.opt	2008-03-11 13:25:23
> +08:00
> > > @@ -0,0 +1 @@
> > > +--replicate-same-server-id --relay-log=slave-relay-bin --skip-slave-start
> > > diff -Nrup a/mysql-test/suite/binlog/t/binlog_auto_increment_bug33029.test
> b/mysql-test/suite/binlog/t/binlog_auto_increment_bug33029.test
> > > --- /dev/null	Wed Dec 31 16:00:00 196900
> > > +++
> b/mysql-test/suite/binlog/t/binlog_auto_increment_bug33029.test	2008-03-11 13:25:23
> +08:00
> > > @@ -0,0 +1,24 @@
> > > +# BUG#33029 5.0 to 5.1 replication fails on dup key when inserting
> > > +# using a trig in SP
> > > +
> > > +# Test if the slave can replicate from a buggy master
> > > +
> > > +copy_file std_data/bug33029-slave-relay-bin.000001
> var/master-data/slave-relay-bin.000001;
> > > +
> > > +write_file var/master-data/slave-relay-bin.index;
> > > +slave-relay-bin.000001
> > > +EOF
> > > +
> > > +change master to
> > > +  MASTER_HOST='dummy.localdomain',
> > > +  RELAY_LOG_FILE='slave-relay-bin.000001',
> > > +  RELAY_LOG_POS=4;
> > > +
> > > +start slave sql_thread;
> > > +disable_result_log;
> > > +select MASTER_POS_WAIT('master-bin.000001', 3776);
> > > +enable_result_log;
> > > +
> > > +echo # Result on slave;
> > > +SELECT * FROM t1;
> > > +SELECT * FROM t2;
> > > diff -Nrup a/mysql-test/suite/rpl/r/rpl_auto_increment_bug33029.result
> b/mysql-test/suite/rpl/r/rpl_auto_increment_bug33029.result
> > > --- /dev/null	Wed Dec 31 16:00:00 196900
> > > +++ b/mysql-test/suite/rpl/r/rpl_auto_increment_bug33029.result	2008-03-11
> 13:25:23 +08:00
> > > @@ -0,0 +1,168 @@
> > > +stop slave;
> > > +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> > > +reset master;
> > > +reset slave;
> > > +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> > > +start slave;
> > > +DROP TABLE IF EXISTS t1, t2;
> > > +DROP PROCEDURE IF EXISTS p1;
> > > +DROP PROCEDURE IF EXISTS p2;
> > > +DROP TRIGGER IF EXISTS tr1;
> > > +DROP FUNCTION IF EXISTS f1;
> > > +CREATE TABLE t1 (id INT AUTO_INCREMENT PRIMARY KEY);
> > > +CREATE TABLE t2 (id INT AUTO_INCREMENT PRIMARY KEY);
> > > +CREATE PROCEDURE p1()
> > > +BEGIN
> > > +DECLARE ins_count INT DEFAULT 10; 
> > > +WHILE ins_count > 0 DO
> > > +INSERT INTO t1 VALUES (NULL);
> > > +SET ins_count = ins_count - 1;
> > > +END WHILE;
> > > +DELETE FROM t1 WHERE id = 1;
> > > +DELETE FROM t1 WHERE id = 2;
> > > +DELETE FROM t2 WHERE id = 1;
> > > +DELETE FROM t2 WHERE id = 2;
> > > +END//
> > > +CREATE PROCEDURE p2()
> > > +BEGIN
> > > +INSERT INTO t1 VALUES (NULL);
> > > +DELETE FROM t1 WHERE id = f1(3);
> > > +DELETE FROM t1 WHERE id = f1(4);
> > > +DELETE FROM t2 WHERE id = 3;
> > > +DELETE FROM t2 WHERE id = 4;
> > > +END//
> > > +CREATE TRIGGER tr1 BEFORE DELETE
> > > +ON t1 FOR EACH ROW 
> > > +BEGIN
> > > +INSERT INTO t2  VALUES (NULL);
> > > +END//
> > > +CREATE FUNCTION f1 (i int) RETURNS int
> > > +BEGIN
> > > +INSERT INTO t2 VALUES (NULL);
> > > +RETURN i;
> > > +END//
> > > +CALL p1();
> > > +show binlog events from <binlog_start>;
> > > +Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=1
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=2
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=3
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=4
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=5
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=6
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=7
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=8
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=9
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=10
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=1
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t1 WHERE id = 1
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=2
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t1 WHERE id = 2
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t2 WHERE id = 1
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t2 WHERE id = 2
> > > +# Result on master
> > > +SELECT * FROM t1;
> > > +id
> > > +3
> > > +4
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +SELECT * FROM t2;
> > > +id
> > > +# Result on slave
> > > +SELECT * FROM t1;
> > > +id
> > > +3
> > > +4
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +SELECT * FROM t2;
> > > +id
> > > +DROP TRIGGER tr1;
> > > +CALL p2();
> > > +show binlog events from <binlog_start>;
> > > +Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=11
> > > +master-bin.000001	#	Query	#	#	use `test`; INSERT INTO t1 VALUES (NULL)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=3
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t1 WHERE id = f1(3)
> > > +master-bin.000001	#	Intvar	#	#	INSERT_ID=12
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t1 WHERE id = f1(4)
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t2 WHERE id = 3
> > > +master-bin.000001	#	Query	#	#	use `test`; DELETE FROM t2 WHERE id = 4
> > > +# Result on master
> > > +SELECT * FROM t1;
> > > +id
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +11
> > > +SELECT * FROM t2;
> > > +id
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +11
> > > +12
> > > +13
> > > +14
> > > +15
> > > +16
> > > +17
> > > +18
> > > +19
> > > +# Result on slave
> > > +SELECT * FROM t1;
> > > +id
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +11
> > > +SELECT * FROM t2;
> > > +id
> > > +5
> > > +6
> > > +7
> > > +8
> > > +9
> > > +10
> > > +11
> > > +12
> > > +13
> > > +14
> > > +15
> > > +16
> > > +17
> > > +18
> > > +19
> > > +DROP TABLE IF EXISTS t1, t2;
> > > +DROP PROCEDURE IF EXISTS p1;
> > > +DROP PROCEDURE IF EXISTS p2;
> > > +DROP FUNCTION IF EXISTS f1;
> > 
> > Have not you forgotten to update the check function - find your TODO?
> > 
> 
> Sorry, I don't understand, maybe I forget something we discussed...
> 
> > > diff -Nrup a/sql/slave.cc b/sql/slave.cc
> > > --- a/sql/slave.cc	2008-02-21 05:24:58 +08:00
> > > +++ b/sql/slave.cc	2008-03-11 13:25:22 +08:00
> > > @@ -4085,6 +4085,28 @@ bool rpl_master_has_bug(Relay_log_info *
> > >    return FALSE;
> > >  }
> > >  
> > > +/**
> > > +   Detect if the master affected by BUG#33029, which does not support
> > > +   using SET INSERT to set auto_increment field value for
> > > +   sub-statements.
> > > + */
> > > +bool rpl_master_bug33029(THD *thd)
> > > +{
> > > +  if (active_mi && active_mi->rli.sql_thd == thd)
> > > +  {
> > > +    Relay_log_info *rli= &active_mi->rli;
> > > +    const uchar *master_ver=
> > > +     
> rli->relay_log.description_event_for_exec->server_version_split;
> > > +    
> > > +    /* TODO: currently for all 5.0 versions, there may be other
> > > +       affected versions that should be checked here.
> > > +     */
> > > +    if (master_ver[0] == 5 && master_ver[1] == 0)
> > > +      return TRUE;
> > > +  }
> > > +  return FALSE;
> > > +}
> > 
> > I thought we agreed that you will enforce the test with checking the
> > minor number that is master_ver[2]. Otherwise I don't get how it
> > is supposed to distinguish 5.1.[0-11] + 5.0.*  from  the rest 
> > 5.1.12+ correct insert_id generating master.
> > 
> 
> Yes, you're right, shamed about this, will be fixed.
> 
> > 
> > > +
> > >  #ifdef HAVE_EXPLICIT_TEMPLATE_INSTANTIATION
> > >  template class I_List_iterator<i_string>;
> > >  template class I_List_iterator<i_string_pair>;
> > > diff -Nrup a/sql/slave.h b/sql/slave.h
> > > --- a/sql/slave.h	2008-02-07 04:07:43 +08:00
> > > +++ b/sql/slave.h	2008-03-11 13:25:22 +08:00
> > > @@ -166,6 +166,7 @@ int fetch_master_table(THD* thd, const c
> > >  bool show_master_info(THD* thd, Master_info* mi);
> > >  bool show_binlog_info(THD* thd);
> > >  bool rpl_master_has_bug(Relay_log_info *rli, uint bug_id);
> > > +bool rpl_master_bug33029(THD *thd);
> > >  
> > >  const char *print_slave_db_safe(const char *db);
> > >  int check_expected_error(THD* thd, Relay_log_info const *rli, int
> error_code);
> > > diff -Nrup a/sql/sql_class.cc b/sql/sql_class.cc
> > > --- a/sql/sql_class.cc	2008-02-13 01:21:07 +08:00
> > > +++ b/sql/sql_class.cc	2008-03-11 13:25:22 +08:00
> > > @@ -28,6 +28,7 @@
> > >  #include "mysql_priv.h"
> > >  #include "rpl_rli.h"
> > >  #include "rpl_record.h"
> > > +#include "slave.h"
> > >  #include <my_bitmap.h>
> > >  #include "log_event.h"
> > >  #include <m_ctype.h>
> > > @@ -2827,6 +2828,11 @@ extern "C" void thd_mark_transaction_to_
> > >  void THD::reset_sub_statement_state(Sub_statement_state *backup,
> > >                                      uint new_state)
> > >  {
> > > +#ifndef EMBEDDED_LIBRARY
> > > +  if (rpl_master_bug33029(this))
> > > +   
> auto_inc_intervals_forced.save(&backup->auto_inc_intervals_forced);
> > > +#endif
> > > +  
> > >    backup->options=         options;
> > >    backup->in_sub_stmt=     in_sub_stmt;
> > >    backup->enable_slow_log= enable_slow_log;
> > > @@ -2864,6 +2870,11 @@ void THD::reset_sub_statement_state(Sub_
> > >  
> > >  void THD::restore_sub_statement_state(Sub_statement_state *backup)
> > >  {
> > > +#ifndef EMBEDDED_LIBRARY
> > > +  if (rpl_master_bug33029(this))
> > > +   
> auto_inc_intervals_forced.restore(&backup->auto_inc_intervals_forced);
> > > +#endif
> > > +
> > >    /*
> > >      To save resources we want to release savepoints which were created
> > >      during execution of function or trigger before leaving their
> savepoint
> > > diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
> > > --- a/sql/sql_class.h	2008-02-20 22:22:13 +08:00
> > > +++ b/sql/sql_class.h	2008-03-11 13:25:23 +08:00
> > > @@ -911,6 +911,7 @@ public:
> > >    ulonglong first_successful_insert_id_in_prev_stmt;
> > >    ulonglong first_successful_insert_id_in_cur_stmt,
> insert_id_for_cur_row;
> > >    Discrete_interval auto_inc_interval_for_cur_row;
> > > +  Discrete_intervals_list auto_inc_intervals_forced;
> > >    ulonglong limit_found_rows;
> > >    ha_rows    cuted_fields, sent_row_count, examined_row_count;
> > >    ulong client_capabilities;
> > > diff -Nrup a/sql/structs.h b/sql/structs.h
> > > --- a/sql/structs.h	2007-12-15 01:01:47 +08:00
> > > +++ b/sql/structs.h	2008-03-11 13:25:23 +08:00
> > 
> > 
> > > @@ -331,6 +331,28 @@ public:
> > >      }
> > >      empty_no_free();
> > >    }
> > > +
> > > +  /* Transfer elements from one list to an empty list */
> > > +  static void transfer(Discrete_intervals_list *from,
> Discrete_intervals_list *to)
> > > +  {
> > > +    DBUG_ASSERT(to->elements == 0);
> > > +    to->head = from->head;
> > > +    to->tail = from->tail;
> > > +    to->current = from->current;
> > > +    to->elements = from->elements;
> > > +    from->empty_no_free();
> > > +  }
> > > +
> > > +  void save(Discrete_intervals_list *to)
> > > +  {
> > > +    transfer(this, to);
> > > +  }
> > > +
> > > +  void restore(Discrete_intervals_list *from)
> > > +  {
> > > +    transfer(from, this);
> > > +  }
> > > +
> > >    const Discrete_interval* get_next()
> > >    {
> > >      Discrete_interval *tmp= current;
> > 
> > Why not to be content with the existing default copy constructor
> > instead of your new methods?
> > 
> > E.g
> > 
> >   auto_inc_intervals_forced.save(&backup->auto_inc_intervals_forced);
> > 
> > turns to be 2 instuctions
> > 
> >   backup->auto_inc_intervals_forced= auto_inc_intervals_forced;
> >   auto_inc_intervals_forced.empty_no_free();
> > 
> > but i'd rather to go with them as empty_no_free() is a public.
> > 
> 
> There is no copy constructor, only a defalut constructor that takes no
> argument, correct me if I am wrong.
> 
> I had thought of use the code you mentioned, the problem is I think a
> simple copy construct that just copy all the members is not safe, I mean
> after:
> 
>    backup->auto_inc_intervals_forced= auto_inc_intervals_forced;
> 
> We will have two instance with shared list of elements, if we use empty
> on any of the instance, the other will be in a dangling state, all the
> elements are actuall freed. In the current case, this is not a problem,
> because we can call empty_no_free to empty the original instance without
> free. But when we add a new interface, it can be use later by other
> code. So this would be an unsafe interface.
> 
> If we do need the copy construct, we have to add a reference count to
> record how many copies we have, and only free the list when the count is
> zero. Maybe we should try this.

Please ignore this, we don't need a record count to do this, I will
commit a new patch soon

> 
> BTW: I think empty_no_free should be private instead of public.
> 
> 
> 
> > 
> > cheers,
> > 
> > Andrei
>