Merge lp:~sergei.glushchenko/percona-server/5.5-ps-bug1192354 into lp:percona-server/5.5

[Sergei Glushchenko]

Bug 1192354: accessing information_schema.partitions causes plans to change
ha_innodb::info stores statistics for keys in the table structure, which is
shared for all the partitions of the table.
When ha_partition::info is invoked, it calls ha_innodb::info for every
partition, determines which has the maximum number of rows and invokes
ha_innodb::info again for this partition. It sets partitioned table
statistics to the statistics of the largest part.
When i_s.partitions is invoked, it calls ha_innodb::info for every partition
as well, but it doesn't perform the last call to set statistics for the
whole table back. So, the table statistics is set to statistics of the last
query, which is not the best choice in most cases.
This patch implements behavior for i_s.partitions similar to behavior of
ha_partition::info.

[Sergei Glushchenko (sergei.glushchenko)]

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/771/

[Alexey Kopytov (akopytov)]

Sergei,

Looks good in general, just minor comments:

  - the fix is not specific to InnoDB. If you want to have InnoDB-specific
    coverage in the test case, it should have --source include/have_innodb.inc
  - the test case should include have_partition.inc as well
  - the fix should also reset table index stats on errors in get_schema_partitions_record()

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

Hi Alexey,

I have added includes into test case.
As for errors in get_schema_partitions_record, seems they are rare. So much rare, that debug and non-debug builds handle them different. However I addressed the issue the most non-intruisive way. I've inserted invocation of file->info in case of error. This will cause file->info for all partitions, even if they were queried before an error occurred.

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/776/

[Alexey Kopytov (akopytov)]

review: Approve

=== added file 'Percona-Server/mysql-test/r/percona_bug1192354.result'
--- Percona-Server/mysql-test/r/percona_bug1192354.result	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/r/percona_bug1192354.result	2013-06-25 15:57:30 +0000
@@ -0,0 +1,39 @@
+DROP TABLE IF EXISTS t1;
+Warnings:
+Note	1051	Unknown table 't1'
+CREATE TABLE t1 (
+c1 int,
+c2 int,
+PRIMARY KEY (c1, c2)
+) ENGINE = InnoDB
+PARTITION BY LIST (c1)
+(PARTITION p0 VALUES IN (0) ENGINE = InnoDB,
+PARTITION p1 VALUES IN (1) ENGINE = InnoDB);
+INSERT INTO t1 VALUES (0, 0);
+INSERT INTO t1 SELECT 0, c2+1 FROM t1;
+INSERT INTO t1 SELECT 0, c2+2 FROM t1;
+INSERT INTO t1 SELECT 0, c2+4 FROM t1;
+INSERT INTO t1 SELECT 0, c2+8 FROM t1;
+ANALYZE TABLE t1;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+SELECT cardinality FROM information_schema.statistics WHERE table_name = 't1';
+cardinality
+2
+17
+SELECT partition_name, table_rows FROM information_schema.partitions WHERE table_name = 't1';
+partition_name	table_rows
+p0	16
+p1	0
+SELECT cardinality FROM information_schema.statistics WHERE table_name = 't1';
+cardinality
+2
+17
+ANALYZE TABLE t1;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+SELECT cardinality FROM information_schema.statistics WHERE table_name = 't1';
+cardinality
+2
+17
+DROP TABLE t1;

=== added file 'Percona-Server/mysql-test/t/percona_bug1192354.test'
--- Percona-Server/mysql-test/t/percona_bug1192354.test	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/t/percona_bug1192354.test	2013-06-25 15:57:30 +0000
@@ -0,0 +1,38 @@
+##############################################################################
+# Bug 1192354: accessing information_schema.partitions causes plans to change
+# MySQL bug:   http://bugs.mysql.com/bug.php?id=69179
+#
+#              This test creates partitioned table and quries it's statistics
+#              before and after querying information_schema.partitions
+##############################################################################
+
+--source include/have_innodb.inc
+--source include/have_partition.inc
+
+DROP TABLE IF EXISTS t1;
+
+CREATE TABLE t1 (
+  c1 int,
+  c2 int,
+  PRIMARY KEY (c1, c2)
+) ENGINE = InnoDB
+PARTITION BY LIST (c1)
+(PARTITION p0 VALUES IN (0) ENGINE = InnoDB,
+ PARTITION p1 VALUES IN (1) ENGINE = InnoDB);
+
+INSERT INTO t1 VALUES (0, 0);
+INSERT INTO t1 SELECT 0, c2+1 FROM t1;
+INSERT INTO t1 SELECT 0, c2+2 FROM t1;
+INSERT INTO t1 SELECT 0, c2+4 FROM t1;
+INSERT INTO t1 SELECT 0, c2+8 FROM t1;
+
+ANALYZE TABLE t1;
+SELECT cardinality FROM information_schema.statistics WHERE table_name = 't1';
+
+SELECT partition_name, table_rows FROM information_schema.partitions WHERE table_name = 't1';
+SELECT cardinality FROM information_schema.statistics WHERE table_name = 't1';
+
+ANALYZE TABLE t1;
+SELECT cardinality FROM information_schema.statistics WHERE table_name = 't1';
+
+DROP TABLE t1;

=== modified file 'Percona-Server/sql/sql_show.cc'
--- Percona-Server/sql/sql_show.cc	2013-06-01 15:04:43 +0000
+++ Percona-Server/sql/sql_show.cc	2013-06-25 15:57:30 +0000
@@ -6145,13 +6145,15 @@
 static void store_schema_partitions_record(THD *thd, TABLE *schema_table,
                                            TABLE *showing_table,
                                            partition_element *part_elem,
-                                           handler *file, uint part_id)
+                                           handler *file, uint part_id,
+                                           ha_rows *records)
 {
   TABLE* table= schema_table;
   CHARSET_INFO *cs= system_charset_info;
   PARTITION_STATS stat_info;
   MYSQL_TIME time;
   file->get_dynamic_partition_info(&stat_info, part_id);
+  *records= stat_info.records;
   table->field[0]->store(STRING_WITH_LEN("def"), cs);
   table->field[12]->store((longlong) stat_info.records, TRUE);
   table->field[13]->store((longlong) stat_info.mean_rec_length, TRUE);
@@ -6274,8 +6276,12 @@
   String tmp_str;
   TABLE *show_table= tables->table;
   handler *file;
+  ha_rows records;
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   partition_info *part_info;
+  uint handler_part_id= 0;
+  ha_rows max_records= 0;
+  PARTITION_STATS stat_info;
 #endif
   DBUG_ENTER("get_schema_partitions_record");
 
@@ -6403,6 +6409,8 @@
                                                list_value,
                                                tmp_str))
           {
+            file->info(HA_STATUS_CONST | HA_STATUS_TIME | HA_STATUS_VARIABLE |
+                       HA_STATUS_VARIABLE_EXTRA | HA_STATUS_NO_LOCK);
             DBUG_RETURN(1);
           }
           table->field[11]->store(tmp_str.ptr(), tmp_str.length(), cs);
@@ -6441,6 +6449,8 @@
                                                  list_value,
                                                  tmp_str))
             {
+              file->info(HA_STATUS_CONST | HA_STATUS_TIME | HA_STATUS_VARIABLE |
+                         HA_STATUS_VARIABLE_EXTRA | HA_STATUS_NO_LOCK);
               DBUG_RETURN(1);
             }
             if (part_info->part_field_list.elements > 1U)
@@ -6477,27 +6487,39 @@
           table->field[6]->set_notnull();
           
           store_schema_partitions_record(thd, table, show_table, subpart_elem,
-                                         file, part_id);
+                                         file, part_id, &records);
+          handler_part_id= (records > max_records) ? part_id : handler_part_id;
           part_id++;
           if(schema_table_store_record(thd, table))
+          {
+            file->info(HA_STATUS_CONST | HA_STATUS_TIME | HA_STATUS_VARIABLE |
+                       HA_STATUS_VARIABLE_EXTRA | HA_STATUS_NO_LOCK);
             DBUG_RETURN(1);
+          }
         }
       }
       else
       {
         store_schema_partitions_record(thd, table, show_table, part_elem,
-                                       file, part_id);
+                                       file, part_id, &records);
+        handler_part_id= (records > max_records) ? part_id : handler_part_id;
         part_id++;
         if(schema_table_store_record(thd, table))
+        {
+          file->info(HA_STATUS_CONST | HA_STATUS_TIME | HA_STATUS_VARIABLE |
+                     HA_STATUS_VARIABLE_EXTRA | HA_STATUS_NO_LOCK);
           DBUG_RETURN(1);
+        }
       }
     }
+    file->get_dynamic_partition_info(&stat_info, handler_part_id);
     DBUG_RETURN(0);
   }
   else
 #endif
   {
-    store_schema_partitions_record(thd, table, show_table, 0, file, 0);
+    store_schema_partitions_record(thd, table, show_table, 0, file, 0,
+                                   &records);
     if(schema_table_store_record(thd, table))
       DBUG_RETURN(1);
   }