bzr commit into mysql-5.1 branch (mattias.jonsson:2679) Bug#39084

[Mattias Jonsson]

#At file:///Users/mattiasj/clones/bzrroot/b39084-51-bugteam/

 2679 Mattias Jonsson	2008-10-29
      Bug#39084: Getting intermittent errors with statement-based binary logging
      
      Problem was that partitioning cached the table flags.
      These flags could change due to TRANSACTION LEVEL changes.
      
      Solution was to remove the cache and always return the table flags
      from the first partition (if the handler was initialized).
added:
  mysql-test/r/partition_innodb_stmt.result
  mysql-test/t/partition_innodb_stmt.test
modified:
  sql/ha_partition.cc
  sql/ha_partition.h

per-file messages:
  mysql-test/r/partition_innodb_stmt.result
    Bug#39084: Getting intermittent errors with statement-based binary logging
    
    New test result file
  mysql-test/t/partition_innodb_stmt.test
    Bug#39084: Getting intermittent errors with statement-based binary logging
    
    New test file
  sql/ha_partition.cc
    Bug#39084: Getting intermittent errors with statement-based binary logging
    
    Removed m_table_flags, and added m_handler_status.
    
    Moved some variable initializations.
    
    Updated some comments.
    
    Added checks that all partitions have the same
    table flags.
  sql/ha_partition.h
    Bug#39084: Getting intermittent errors with statement-based binary logging
    
    Removed m_table_flags, and added m_handler_status.
    
    Updated some comments.
    
    Always return the first partitions table flags, instead of using
    cached table flags.
=== added file 'mysql-test/r/partition_innodb_stmt.result'
--- a/mysql-test/r/partition_innodb_stmt.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/partition_innodb_stmt.result	2008-10-29 11:39:08 +0000
@@ -0,0 +1,30 @@
+#connection default
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+CREATE TABLE t1
+(
+id SMALLINT NOT NULL,
+PRIMARY KEY (id)
+) ENGINE=innodb
+PARTITION BY RANGE (id)
+(
+PARTITION p1 VALUES LESS THAN (2),
+PARTITION p2 VALUES LESS THAN (4),
+PARTITION p3 VALUES LESS THAN (10)
+);
+FLUSH TABLES;
+INSERT INTO t1 VALUES (1),(2),(3);
+FLUSH TABLES;
+SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
+BEGIN;
+SELECT * FROM t1;
+id
+1
+2
+3
+#connection con1
+SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+INSERT INTO t1 VALUES(7);
+COMMIT;
+COMMIT;
+DROP TABLE t1;

=== added file 'mysql-test/t/partition_innodb_stmt.test'
--- a/mysql-test/t/partition_innodb_stmt.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/partition_innodb_stmt.test	2008-10-29 11:39:08 +0000
@@ -0,0 +1,39 @@
+--source include/have_binlog_format_statement.inc
+--source include/have_innodb.inc
+
+--echo #connection default
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+
+CREATE TABLE t1
+(
+  id SMALLINT NOT NULL,
+  PRIMARY KEY (id)
+) ENGINE=innodb
+PARTITION BY RANGE (id)
+(
+  PARTITION p1 VALUES LESS THAN (2),
+  PARTITION p2 VALUES LESS THAN (4),
+  PARTITION p3 VALUES LESS THAN (10)
+);
+
+FLUSH TABLES;
+INSERT INTO t1 VALUES (1),(2),(3);
+
+FLUSH TABLES;
+SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
+BEGIN;
+SELECT * FROM t1;
+
+connect (con1, localhost, root,,);
+connection con1;
+
+--echo #connection con1
+SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+BEGIN;
+INSERT INTO t1 VALUES(7);
+COMMIT;
+
+disconnect con1;
+connection default;
+COMMIT;
+DROP TABLE t1;

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2008-10-06 13:14:20 +0000
+++ b/sql/ha_partition.cc	2008-10-29 11:39:08 +0000
@@ -160,8 +160,7 @@ const uint ha_partition::NO_CURRENT_PART
 
 ha_partition::ha_partition(handlerton *hton, TABLE_SHARE *share)
   :handler(hton, share), m_part_info(NULL), m_create_handler(FALSE),
-   m_is_sub_partitioned(0), is_clone(FALSE), auto_increment_lock(FALSE),
-   auto_increment_safe_stmt_log_lock(FALSE)
+   m_is_sub_partitioned(0)
 {
   DBUG_ENTER("ha_partition::ha_partition(table)");
   init_handler_variables();
@@ -181,10 +180,8 @@ ha_partition::ha_partition(handlerton *h
 */
 
 ha_partition::ha_partition(handlerton *hton, partition_info *part_info)
-  :handler(hton, NULL), m_part_info(part_info),
-   m_create_handler(TRUE),
-   m_is_sub_partitioned(m_part_info->is_sub_partitioned()), is_clone(FALSE),
-   auto_increment_lock(FALSE), auto_increment_safe_stmt_log_lock(FALSE)
+  :handler(hton, NULL), m_part_info(part_info), m_create_handler(TRUE),
+   m_is_sub_partitioned(m_part_info->is_sub_partitioned())
 {
   DBUG_ENTER("ha_partition::ha_partition(part_info)");
   init_handler_variables();
@@ -231,7 +228,7 @@ void ha_partition::init_handler_variable
   m_innodb= FALSE;
   m_extra_cache= FALSE;
   m_extra_cache_size= 0;
-  m_table_flags= HA_FILE_BASED | HA_REC_NOT_IN_SEQ;
+  m_handler_status= handler_not_initialized;
   m_low_byte_first= 1;
   m_part_field_array= NULL;
   m_ordered_rec_buffer= NULL;
@@ -240,6 +237,9 @@ void ha_partition::init_handler_variable
   m_last_part= 0;
   m_rec0= 0;
   m_curr_key_info= 0;
+  is_clone= FALSE,
+  auto_increment_lock= FALSE;
+  auto_increment_safe_stmt_log_lock= FALSE;
   /*
     this allows blackhole to work properly
   */
@@ -333,6 +333,7 @@ ha_partition::~ha_partition()
 bool ha_partition::initialise_partition(MEM_ROOT *mem_root)
 {
   handler **file_array, *file;
+  ulonglong check_table_flags;
   DBUG_ENTER("ha_partition::initialise_partition");
 
   if (m_create_handler)
@@ -345,11 +346,9 @@ bool ha_partition::initialise_partition(
   else if (!table_share || !table_share->normalized_path.str)
   {
     /*
-      Called with dummy table share (delete, rename and alter table)
-      Don't need to set-up table flags other than
-      HA_FILE_BASED here
+      Called with dummy table share (delete, rename and alter table).
+      Don't need to set-up anything.
     */
-    m_table_flags|= HA_FILE_BASED | HA_REC_NOT_IN_SEQ;
     DBUG_RETURN(0);
   }
   else if (get_from_handler_file(table_share->normalized_path.str, mem_root))
@@ -361,15 +360,12 @@ bool ha_partition::initialise_partition(
     We create all underlying table handlers here. We do it in this special
     method to be able to report allocation errors.
 
-    Set up table_flags, low_byte_first, primary_key_is_clustered and
+    Set up low_byte_first, primary_key_is_clustered and
     has_transactions since they are called often in all kinds of places,
     other parameters are calculated on demand.
-    HA_FILE_BASED is always set for partition handler since we use a
-    special file for handling names of partitions, engine types.
-    HA_CAN_GEOMETRY, HA_CAN_FULLTEXT, HA_CAN_SQL_HANDLER, HA_DUPLICATE_POS,
-    HA_CAN_INSERT_DELAYED is disabled until further investigated.
+    Verify that all partitions have the same table_flags.
   */
-  m_table_flags= (ulong)m_file[0]->ha_table_flags();
+  check_table_flags= m_file[0]->ha_table_flags();
   m_low_byte_first= m_file[0]->low_byte_first();
   m_pkey_is_clustered= TRUE;
   file_array= m_file;
@@ -384,12 +380,13 @@ bool ha_partition::initialise_partition(
     }
     if (!file->primary_key_is_clustered())
       m_pkey_is_clustered= FALSE;
-    m_table_flags&= file->ha_table_flags();
+    if (check_table_flags != file->ha_table_flags())
+    {
+      my_error(ER_MIX_HANDLER_ERROR, MYF(0));
+      DBUG_RETURN(1);
+    }
   } while (*(++file_array));
-  m_table_flags&= ~(HA_CAN_GEOMETRY | HA_CAN_FULLTEXT | HA_DUPLICATE_POS |
-                    HA_CAN_SQL_HANDLER | HA_CAN_INSERT_DELAYED |
-                    HA_PRIMARY_KEY_REQUIRED_FOR_POSITION);
-  m_table_flags|= HA_FILE_BASED | HA_REC_NOT_IN_SEQ;
+  m_handler_status= handler_initialized;
   DBUG_RETURN(0);
 }
 
@@ -2410,6 +2407,7 @@ int ha_partition::open(const char *name,
   handler **file;
   char name_buff[FN_REFLEN];
   bool is_not_tmp_table= (table_share->tmp_table == NO_TMP_TABLE);
+  ulonglong check_table_flags;
   DBUG_ENTER("ha_partition::open");
 
   DBUG_ASSERT(table->s == table_share);
@@ -2458,7 +2456,7 @@ int ha_partition::open(const char *name,
   }
 
   /* Recalculate table flags as they may change after open */
-  m_table_flags= m_file[0]->ha_table_flags();
+  check_table_flags= m_file[0]->ha_table_flags();
   file= m_file;
   do
   {
@@ -2470,11 +2468,9 @@ int ha_partition::open(const char *name,
     m_no_locks+= (*file)->lock_count();
     name_buffer_ptr+= strlen(name_buffer_ptr) + 1;
     set_if_bigger(ref_length, ((*file)->ref_length));
-    m_table_flags&= (*file)->ha_table_flags();
+    if (check_table_flags != (*file)->ha_table_flags())
+      goto err_handler;
   } while (*(++file));
-  m_table_flags&= ~(HA_CAN_GEOMETRY | HA_CAN_FULLTEXT | HA_DUPLICATE_POS |
-                    HA_CAN_SQL_HANDLER | HA_CAN_INSERT_DELAYED);
-  m_table_flags|= HA_FILE_BASED | HA_REC_NOT_IN_SEQ;
   key_used_on_scan= m_file[0]->key_used_on_scan;
   implicit_emptied= m_file[0]->implicit_emptied;
   /*
@@ -2526,6 +2522,7 @@ int ha_partition::open(const char *name,
     calling open on all individual handlers.
   */
   info(HA_STATUS_VARIABLE | HA_STATUS_CONST);
+  m_handler_status= handler_opened;
   DBUG_RETURN(0);
 
 err_handler:
@@ -2595,6 +2592,7 @@ repeat:
     goto repeat;
   }
 
+  m_handler_status= handler_closed;
   DBUG_RETURN(0);
 }
 
@@ -4763,7 +4761,7 @@ int ha_partition::info(uint flag)
       }
     } while (*(++file_array));
     if (stats.records < 2 &&
-        !(m_table_flags & HA_STATS_RECORDS_IS_EXACT))
+        !(m_file[0]->ha_table_flags() & HA_STATS_RECORDS_IS_EXACT))
       stats.records= 2;
     if (stats.records > 0)
       stats.mean_rec_length= (ulong) (stats.data_file_length / stats.records);

=== modified file 'sql/ha_partition.h'
--- a/sql/ha_partition.h	2008-10-01 10:14:55 +0000
+++ b/sql/ha_partition.h	2008-10-29 11:39:08 +0000
@@ -85,8 +85,15 @@ private:
     for this since the MySQL Server sometimes allocating the handler object
     without freeing them.
   */
-  longlong m_table_flags;
   ulong m_low_byte_first;
+  enum enum_handler_status
+  {
+    handler_not_initialized= 0,
+    handler_initialized,
+    handler_opened,
+    handler_closed
+  };
+  enum_handler_status m_handler_status;
 
   uint m_reorged_parts;                  // Number of reorganised parts
   uint m_tot_parts;                      // Total number of partitions;
@@ -585,6 +592,8 @@ public:
     The partition handler will support whatever the underlying handlers
     support except when specifically mentioned below about exceptions
     to this rule.
+    NOTE: This cannot be cached since it can depend on TRANSACTION ISOLATION
+    LEVEL which is dynamic, see bug#39084.
 
     HA_READ_RND_SAME:
     Not currently used. (Means that the handler supports the rnd_same() call)
@@ -709,9 +718,34 @@ public:
     transfer those calls into index_read and other calls in the
     index scan module.
     (NDB)
+
+    HA_PRIMARY_KEY_REQUIRED_FOR_POSITION:
+    Does the storage engine need a PK for position?
+    Used with hidden primary key in InnoDB.
+    Hidden primary keys annot be supported by partitioning, since the
+    partitioning expressions columns must be a part of the primary key.
+    (InnoDB)
+
+    HA_FILE_BASED is always set for partition handler since we use a
+    special file for handling names of partitions, engine types.
+    HA_REC_NOT_IN_SEQ is always set for partition handler since we cannot
+    guarantee that the records will be returned in sequence.
+    HA_CAN_GEOMETRY, HA_CAN_FULLTEXT, HA_CAN_SQL_HANDLER, HA_DUPLICATE_POS,
+    HA_CAN_INSERT_DELAYED, HA_PRIMARY_KEY_REQUIRED_FOR_POSITION is disabled
+    until further investigated.
   */
-  virtual ulonglong table_flags() const
-  { return m_table_flags; }
+  virtual Table_flags table_flags() const
+  {
+    if (m_handler_status < handler_initialized ||
+        m_handler_status >= handler_closed)
+      return (HA_FILE_BASED | HA_REC_NOT_IN_SEQ);
+    else
+      return (m_file[0]->ha_table_flags() &
+              ~(HA_CAN_GEOMETRY | HA_CAN_FULLTEXT | HA_DUPLICATE_POS |
+                HA_CAN_SQL_HANDLER | HA_CAN_INSERT_DELAYED |
+                HA_PRIMARY_KEY_REQUIRED_FOR_POSITION)) |
+             (HA_FILE_BASED | HA_REC_NOT_IN_SEQ);
+  }
 
   /*
     This is a bitmap of flags that says how the storage engine