MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Georgi Kodinov Date:July 2 2009 2:42pm
Subject:bzr commit into mysql-5.1-bugteam branch (joro:2988) Bug#45807
View as plain text  
#At file:///home/kgeorge/mysql/work/B45807-5.1-bugteam/ based on revid:staale.smedseng@stripped

 2988 Georgi Kodinov	2009-07-02
      Bug #45807: crash accessing partitioned table and sql_mode
      contains ONLY_FULL_GROUP_BY
      
      The partitioning code needs to issue a Item::fix_fields()
      on the partitioning expression in order to prepare 
      it for being evaluated.
      It does this by creating a special table and a table list 
      for the scope of the partitioning expression.
      But when checking ONLY_FULL_GROUP_BY the 
      Item_field::fix_fields() was relying that there always be
      cached_table set and was trying to use it to get the 
      select_lex of the SELECT the field's table is in.
      But the cached_table was not set by the partitioning code
      that creates the artificial TABLE_LIST used to resolve the
      partitioning expression and this resulted in a crash.
       
      Fixed by rectifying the following errors :
      1. Item_field::fix_fields() : the code that check for 
      ONLY_FULL_GROUP_BY relies on having tables with 
      cacheable_table set. This is mostly true, the only 
      two exceptions being the partitioning context table
      and the trigger context table.
      Fixed by taking the current parsing context if no pointer
      to the TABLE_LIST instance is present in the cached_table.
      
      2. fix_fields_part_func() : 
      
      2a. The code that adds the table being created to the 
      scope for the partitioning expression is mostly a copy 
      of the add_table_to_list and friends with one exception :
      it was not marking the table as cacheable (something that
      normal add_table_to_list is doing). This caused the 
      problem in the check for ONLY_FULL_GROUP_BY in 
      Item_field::fix_fields() to appear.
      Fixed by setting the correct members to make the table
      cacheable.
      The ideal structural fix for this is to use a unified 
      interface for adding a table to a table list 
      (add_table_to_list?) : noted in a TODO comment
      
      2b. The Item::fix_fields() was called with a NULL destination
      pointer. This causes uninitalized memory reads in the 
      overloaded ::fix_fields() function (namely 
      Item_field::fix_fields()) as it expects a non-zero pointer 
      there. Fixed by passing the source pointer similarly to how 
      it's done in JOIN::prepare().
     @ mysql-test/r/partition.result
        Bug #45807: test case
     @ mysql-test/t/partition.test
        Bug #45807: test case
     @ sql/item.cc
        Bug #45807: fix the ONLY_FULL_GROUP_BY check code to 
        handle correctly non-cacheable tables.
     @ sql/sql_partition.cc
        Bug #45807: fix the Item::fix_fields() context
        initializatio for the partitioning expression in 
        CREATE TABLE.

    modified:
      mysql-test/r/partition.result
      mysql-test/t/partition.test
      sql/item.cc
      sql/sql_partition.cc
=== modified file 'mysql-test/r/partition.result'
--- a/mysql-test/r/partition.result	2009-06-16 09:59:57 +0000
+++ b/mysql-test/r/partition.result	2009-07-02 14:42:00 +0000
@@ -1982,5 +1982,14 @@ SELECT b, c FROM t1 WHERE b = 1 GROUP BY
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	range	bc	bc	10	NULL	7	Using where; Using index for group-by
 DROP TABLE t1;
+#
+# Bug #45807: crash accessing partitioned table and sql_mode 
+#   contains ONLY_FULL_GROUP_BY
+#
+SET SESSION SQL_MODE='ONLY_FULL_GROUP_BY';
+CREATE TABLE t1(id INT,KEY(id)) ENGINE=MYISAM 
+PARTITION BY HASH(id) PARTITIONS 2;
+DROP TABLE t1;
+SET SESSION SQL_MODE=DEFAULT;
 End of 5.1 tests
 SET @@global.general_log= @old_general_log;

=== modified file 'mysql-test/t/partition.test'
--- a/mysql-test/t/partition.test	2009-06-16 09:59:57 +0000
+++ b/mysql-test/t/partition.test	2009-07-02 14:42:00 +0000
@@ -1976,6 +1976,17 @@ SELECT b, c FROM t1 WHERE b = 1 GROUP BY
 
 DROP TABLE t1;
 
+--echo #
+--echo # Bug #45807: crash accessing partitioned table and sql_mode 
+--echo #   contains ONLY_FULL_GROUP_BY
+--echo #
+
+SET SESSION SQL_MODE='ONLY_FULL_GROUP_BY';
+CREATE TABLE t1(id INT,KEY(id)) ENGINE=MYISAM 
+  PARTITION BY HASH(id) PARTITIONS 2;
+DROP TABLE t1;
+SET SESSION SQL_MODE=DEFAULT;
+
 --echo End of 5.1 tests
 
 SET @@global.general_log= @old_general_log;

=== modified file 'sql/item.cc'
--- a/sql/item.cc	2009-06-25 06:22:39 +0000
+++ b/sql/item.cc	2009-07-02 14:42:00 +0000
@@ -4406,16 +4406,22 @@ mark_non_agg_field:
       Fields from outer selects added to the aggregate function
       outer_fields list as its unknown at the moment whether it's
       aggregated or not.
+      We're using either the select lex of the cached table (if present)
+      or the field's resolution context. context->select_lex is 
+      safe for use because it's either the SELECT we want to use 
+      (the current level) or a stub added by non-SELECT queries.
     */
+    SELECT_LEX *select_lex= cached_table ? 
+      cached_table->select_lex : context->select_lex;
     if (!thd->lex->in_sum_func)
-      cached_table->select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
+      select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
     else
     {
       if (outer_fixed)
         thd->lex->in_sum_func->outer_fields.push_back(this);
       else if (thd->lex->in_sum_func->nest_level !=
           thd->lex->current_select->nest_level)
-        cached_table->select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
+        select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
     }
   }
   return FALSE;

=== modified file 'sql/sql_partition.cc'
--- a/sql/sql_partition.cc	2009-06-19 08:24:43 +0000
+++ b/sql/sql_partition.cc	2009-07-02 14:42:00 +0000
@@ -918,6 +918,9 @@ bool fix_fields_part_func(THD *thd, Item
     Set-up the TABLE_LIST object to be a list with a single table
     Set the object to zero to create NULL pointers and set alias
     and real name to table name and get database name from file name.
+    TODO: Consider generalizing or refactoring Lex::add_table_to_list() so
+    it can be used in all places where we create TABLE_LIST objects.
+    Also consider creating appropriate constructors for TABLE_LIST.
   */
 
   bzero((void*)&tables, sizeof(TABLE_LIST));
@@ -925,6 +928,13 @@ bool fix_fields_part_func(THD *thd, Item
   tables.table= table;
   tables.next_local= 0;
   tables.next_name_resolution_table= 0;
+  /*
+    Cache the table in Item_fields. All the tables can be cached except
+    the trigger pseudo table.
+  */
+  tables.cacheable_table= TRUE;
+  context= thd->lex->current_context();
+  tables.select_lex= context->select_lex;
   strmov(db_name_string, table->s->normalized_path.str);
   dir_length= dirname_length(db_name_string);
   db_name_string[dir_length - 1]= 0;
@@ -932,7 +942,6 @@ bool fix_fields_part_func(THD *thd, Item
   db_name= &db_name_string[home_dir_length];
   tables.db= db_name;
 
-  context= thd->lex->current_context();
   table->map= 1; //To ensure correct calculation of const item
   table->get_fields_in_item_tree= TRUE;
   save_table_list= context->table_list;
@@ -965,7 +974,7 @@ bool fix_fields_part_func(THD *thd, Item
   save_use_only_table_context= thd->lex->use_only_table_context;
   thd->lex->use_only_table_context= TRUE;
   
-  error= func_expr->fix_fields(thd, (Item**)0);
+  error= func_expr->fix_fields(thd, (Item**)&func_expr);
 
   thd->lex->use_only_table_context= save_use_only_table_context;
 


Attachment: [text/bzr-bundle] bzr/joro@sun.com-20090702144200-vc1v1n3qhlplckui.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (joro:2988) Bug#45807Georgi Kodinov2 Jul