Bug #14483484 INSERT ... ON DUPLICATE KEY UPDATE + INNODB_AUTOINC_LOCK_MODE=1

gurusami · gurusami · commit 869c1325d15c · 2013-04-16T14:22:38.000+05:30
IS BROKEN

Problem:

When a multi-row INSERT ... ON DUPLICATE KEY UPDATE statement begins
execution it reserves the required number of auto increment values.
When one of the insert fails because of duplicate key error, then it
will become an update operation.  The auto increment value of this
update operation will determine the auto increment values
used for subsequent inserts.  This can result in duplicate auto increment
values.

Solution:

When an insert operation of a multi-row INSERT ... ON DUPLICATE KEY 
UPDATE statement fails, check if the update clause explicitly updates
the auto increment value. If an explicit value is given for an 
auto increment field, then check if it will conflict with the values
that we have reserved.  If it might conflict then return an error.  
Also, the update clause will not bump the auto increment value for
the rows inserted in the multi-row INSERT ... ON DUPLICATE KEY.

rb#2219 approved by Guilhem and Dmitry Shulga.
diff --git a/mysql-test/suite/rpl/r/rpl_insert_id.result b/mysql-test/suite/rpl/r/rpl_insert_id.result
@@ -233,14 +233,14 @@ select * from t1 order by n;
 n	b
 2	100
 4	400
+5	600
 1000	350
-1001	600
 select * from t1 order by n;
 n	b
 2	100
 4	400
+5	600
 1000	350
-1001	600
 drop table t1;
 create table t1 (n int primary key auto_increment not null,
 b int, unique(b)) engine=myisam;
diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
@@ -7057,6 +7057,9 @@ ER_WARN_PURGE_LOG_IN_USE
 ER_WARN_PURGE_LOG_IS_ACTIVE
   eng "file %s was not purged because it is the active log file."
 
+ER_AUTO_INCREMENT_CONFLICT
+  eng "Auto-increment value in UPDATE conflicts with internally generated values"
+
 #
 #  End of 5.6 error messages.
 #
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
@@ -1712,6 +1712,37 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, COPY_INFO *update)
                                                  TRG_EVENT_UPDATE))
           goto before_trg_err;
 
+        bool insert_id_consumed= false;
+        if (// UPDATE clause specifies a value for the auto increment field
+            table->auto_increment_field_not_null &&
+            // An auto increment value has been generated for this row
+            (insert_id_for_cur_row > 0))
+        {
+          // After-update value:
+          const ulonglong auto_incr_val= table->next_number_field->val_int();
+          if (auto_incr_val == insert_id_for_cur_row)
+          {
+            // UPDATE wants to use the generated value
+            insert_id_consumed= true;
+          }
+          else if (table->file->auto_inc_interval_for_cur_row.
+                   in_range(auto_incr_val))
+          {
+            /*
+              UPDATE wants to use one auto generated value which we have already
+              reserved for another (previous or following) row. That may cause
+              a duplicate key error if we later try to insert the reserved
+              value. Such conflicts on auto generated values would be strange
+              behavior, so we return a clear error now.
+            */
+            my_error(ER_AUTO_INCREMENT_CONFLICT, MYF(0));
+	    goto before_trg_err;
+          }
+        }
+
+        if (!insert_id_consumed)
+          table->file->restore_auto_increment(prev_insert_id);
+
         /* CHECK OPTION for VIEW ... ON DUPLICATE KEY UPDATE ... */
         {
           const TABLE_LIST *inserted_view=
@@ -1726,7 +1757,6 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, COPY_INFO *update)
           }
         }
 
-        table->file->restore_auto_increment(prev_insert_id);
         info->stats.touched++;
         if (!records_are_comparable(table) || compare_records(table))
         {
@@ -1763,9 +1793,6 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, COPY_INFO *update)
           info->stats.copied++;
         }
 
-        if (table->next_number_field)
-          table->file->adjust_next_insert_id_after_explicit_value(
-            table->next_number_field->val_int());
         goto ok_or_after_trg_err;
       }
       else /* DUP_REPLACE */
diff --git a/sql/structs.h b/sql/structs.h
@@ -1,7 +1,7 @@
 #ifndef STRUCTS_INCLUDED
 #define STRUCTS_INCLUDED
 
-/* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -292,6 +292,13 @@ class Discrete_interval {
   ulonglong  interval_max;    // excluded bound. Redundant.
 public:
   Discrete_interval *next;    // used when linked into Discrete_intervals_list
+
+  /// Determine if the given value is within the interval
+  bool in_range(const ulonglong value) const
+  {
+    return  ((value >= interval_min) && (value < interval_max));
+  }
+
   void replace(ulonglong start, ulonglong val, ulonglong incr)
   {
     interval_min=    start;

Original file line number	Diff line number	Diff line change
`@@ -7057,6 +7057,9 @@ ER_WARN_PURGE_LOG_IN_USE`
`7057`	`7057`	`ER_WARN_PURGE_LOG_IS_ACTIVE`
`7058`	`7058`	`eng "file %s was not purged because it is the active log file."`
`7059`	`7059`
	`7060`	`+ER_AUTO_INCREMENT_CONFLICT`
	`7061`	`+ eng "Auto-increment value in UPDATE conflicts with internally generated values"`
	`7062`	`+`
`7060`	`7063`	`#`
`7061`	`7064`	`# End of 5.6 error messages.`
`7062`	`7065`	`#`