[Commits] bzr commit into MariaDB 5.1, with Maria 1.5:maria branch (igor:2858) Bug#45640

18 May 2010

#At lp:maria based on revid:sergii@pisem.net-20100515121733-8gfu2ixa3cbqxid3

 2858 Igor Babaev	2010-05-17
      An alternative fix for bug #45640 committed against MariaDB 5.1.44.
      The result file must be adjusted after the fix for bug 40277 is apllied.
      modified:
        mysql-test/r/group_by.result
        mysql-test/t/group_by.test
        sql/item.cc
        sql/item.h
        sql/mysql_priv.h
        sql/sql_select.cc

=== modified file 'mysql-test/r/group_by.result'

--- a/mysql-test/r/group_by.result	2009-02-26 17:17:06 +0000
+++ b/mysql-test/r/group_by.result	2010-05-18 04:03:53 +0000
@@ -1703,3 +1703,91 @@ COUNT(i)
 1
 DROP TABLE t1;
 SET @@sql_mode = @old_sql_mode;
+#
+# Bug #45640: optimizer bug produces wrong results
+#
+CREATE TABLE t1 (a INT, b INT);
+INSERT INTO t1 VALUES (4, 40), (1, 10), (2, 20), (2, 20), (3, 30);
+# should return 4 ordered records:
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa;
+aa	COUNT(DISTINCT b)
+1	1
+2	1
+3	1
+4	1
+SELECT (SELECT (SELECT t1.a)) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa;
+aa	COUNT(DISTINCT b)
+1	1
+2	1
+3	1
+4	1
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa+0;
+aa	COUNT(DISTINCT b)
+1	1
+2	1
+3	1
+4	1
+# should return the same result in a reverse order:
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY -aa;
+aa	COUNT(DISTINCT b)
+4	1
+3	1
+2	1
+1	1
+# execution plan should not use temporary table:
+EXPLAIN EXTENDED
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa+0;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	5	100.00	Using filesort
+2	DEPENDENT SUBQUERY	NULL	NULL	NULL	NULL	NULL	NULL	NULL	NULL	No tables used
+Warnings:
+Note	1276	Field or reference 'test.t1.a' of SELECT #2 was resolved in SELECT #1
+Note	1003	select (select `test`.`t1`.`a` AS `a`) AS `aa`,count(distinct `test`.`t1`.`b`) AS `COUNT(DISTINCT b)` from `test`.`t1` group by ((select `test`.`t1`.`a` AS `a`) + 0)
+EXPLAIN EXTENDED
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY -aa;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	5	100.00	Using filesort
+2	DEPENDENT SUBQUERY	NULL	NULL	NULL	NULL	NULL	NULL	NULL	NULL	No tables used
+Warnings:
+Note	1276	Field or reference 'test.t1.a' of SELECT #2 was resolved in SELECT #1
+Note	1003	select (select `test`.`t1`.`a` AS `a`) AS `aa`,count(distinct `test`.`t1`.`b`) AS `COUNT(DISTINCT b)` from `test`.`t1` group by -((select `test`.`t1`.`a` AS `a`))
+# should return only one record
+SELECT (SELECT tt.a FROM t1 tt LIMIT 1) aa, COUNT(DISTINCT b) FROM t1
+GROUP BY aa;
+aa	COUNT(DISTINCT b)
+4	4
+CREATE TABLE t2 SELECT DISTINCT a FROM t1;
+# originally reported queries (1st two columns of next two query
+# results should be same):
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(DISTINCT b)
+FROM t1 GROUP BY aa, b;
+aa	b	COUNT(DISTINCT b)
+1	10	1
+2	20	1
+3	30	1
+4	40	1
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(         b)
+FROM t1 GROUP BY aa, b;
+aa	b	COUNT(         b)
+1	10	1
+2	20	2
+3	30	1
+4	40	1
+# ORDER BY for sure:
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(DISTINCT b)
+FROM t1 GROUP BY aa, b ORDER BY -aa, -b;
+aa	b	COUNT(DISTINCT b)
+4	40	1
+3	30	1
+2	20	1
+1	10	1
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(         b)
+FROM t1 GROUP BY aa, b ORDER BY -aa, -b;
+aa	b	COUNT(         b)
+4	40	1
+3	30	1
+2	20	2
+1	10	1
+DROP TABLE t1, t2;
+#
+# End of 5.1 tests

=== modified file 'mysql-test/t/group_by.test'
--- a/mysql-test/t/group_by.test	2009-02-26 17:17:06 +0000
+++ b/mysql-test/t/group_by.test	2010-05-18 04:03:53 +0000
@@ -1158,3 +1158,53 @@ SELECT COUNT(i) FROM t1 WHERE i > 1;
 DROP TABLE t1;
 SET @@sql_mode = @old_sql_mode;
 
+--echo #
+--echo # Bug #45640: optimizer bug produces wrong results
+--echo #
+
+CREATE TABLE t1 (a INT, b INT);
+INSERT INTO t1 VALUES (4, 40), (1, 10), (2, 20), (2, 20), (3, 30);
+
+--echo # should return 4 ordered records:
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa;
+
+SELECT (SELECT (SELECT t1.a)) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa;
+
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa+0;
+
+--echo # should return the same result in a reverse order:
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY -aa;
+
+--echo # execution plan should not use temporary table:
+EXPLAIN EXTENDED
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY aa+0;
+
+EXPLAIN EXTENDED
+SELECT (SELECT t1.a) aa, COUNT(DISTINCT b) FROM t1 GROUP BY -aa;
+
+--echo # should return only one record
+SELECT (SELECT tt.a FROM t1 tt LIMIT 1) aa, COUNT(DISTINCT b) FROM t1
+  GROUP BY aa;
+
+CREATE TABLE t2 SELECT DISTINCT a FROM t1;
+
+--echo # originally reported queries (1st two columns of next two query
+--echo # results should be same):
+
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(DISTINCT b)
+  FROM t1 GROUP BY aa, b;
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(         b)
+  FROM t1 GROUP BY aa, b;
+
+--echo # ORDER BY for sure:
+
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(DISTINCT b)
+  FROM t1 GROUP BY aa, b ORDER BY -aa, -b;
+SELECT (SELECT t2.a FROM t2 WHERE t2.a = t1.a) aa, b, COUNT(         b)
+  FROM t1 GROUP BY aa, b ORDER BY -aa, -b;
+
+DROP TABLE t1, t2;
+
+--echo #
+
+--echo # End of 5.1 tests

=== modified file 'sql/item.cc'
--- a/sql/item.cc	2010-03-28 18:10:00 +0000
+++ b/sql/item.cc	2010-05-18 04:03:53 +0000
@@ -4313,13 +4313,16 @@ bool Item_field::fix_fields(THD *thd, It
             Item_ref *rf= new Item_ref(context, db_name,table_name,field_name);
             if (!rf)
               return 1;
-            thd->change_item_tree(reference, rf);
-            /*
-              Because Item_ref never substitutes itself with other items 
-              in Item_ref::fix_fields(), we can safely use the original 
-              pointer to it even after fix_fields()
-             */
-            return rf->fix_fields(thd, reference) ||  rf->check_cols(1);
+            bool ret= rf->fix_fields(thd, (Item **) &rf) || rf->check_cols(1);
+            if (ret)
+              return TRUE;
+           
+            SELECT_LEX *select= thd->lex->current_select;
+            thd->change_item_tree(reference,
+                                  select->parsing_place == IN_GROUP_BY && 
+				  alias_name_used  ?  *rf->ref : rf);
+
+            return FALSE;
           }
         }
       }
@@ -6383,6 +6386,42 @@ bool Item_outer_ref::fix_fields(THD *thd
 
 
 /**
+  Mark references from inner selects used in group by clause
+
+  The method is used by the walk method when called for the expressions
+  from the group by clause. The callsare  occurred in the function
+  fix_inner_refs invoked by JOIN::prepare.
+  The parameter passed to Item_outer_ref::check_inner_refs_processor
+  is the iterator over the list of inner references from the subselects
+  of the select to be prepared. The function marks those references
+  from this list whose occurrences are encountered in the group by 
+  expressions passed to the walk method.  
+ 
+  @param arg  pointer to the iterator over a list of inner references
+
+  @return
+    FALSE always
+*/
+
+bool Item_outer_ref::check_inner_refs_processor(uchar *arg)
+{
+  List_iterator_fast<Item_outer_ref> *it=
+    ((List_iterator_fast<Item_outer_ref> *) arg);
+  Item_outer_ref *ref;
+  while ((ref= (*it)++))
+  {
+    if (ref == this)
+    {
+      ref->found_in_group_by= 1;
+      break;
+    }
+  }
+  (*it).rewind();
+  return FALSE;
+}
+
+
+/**
   Compare two view column references for equality.
 
   A view column reference is considered equal to another column

=== modified file 'sql/item.h'
--- a/sql/item.h	2010-03-30 12:36:49 +0000
+++ b/sql/item.h	2010-05-18 04:03:53 +0000
@@ -997,6 +997,8 @@ public:
     return FALSE;
   }
 
+  virtual bool check_inner_refs_processor(uchar *arg) { return FALSE; }
+
   /*
     For SP local variable returns pointer to Item representing its
     current value and pointer to current Item otherwise.
@@ -2310,7 +2312,10 @@ public:
     return ref ? (*ref)->real_item() : this;
   }
   bool walk(Item_processor processor, bool walk_subquery, uchar *arg)
-  { return (*ref)->walk(processor, walk_subquery, arg); }
+  {
+    return (*ref)->walk(processor, walk_subquery, arg) ||
+           (this->*processor)(arg);
+  }
   virtual void print(String *str, enum_query_type query_type);
   bool result_as_longlong()
   {
@@ -2428,12 +2433,13 @@ public:
     of the outer select.
   */
   bool found_in_select_list;
+  bool found_in_group_by;
   Item_outer_ref(Name_resolution_context *context_arg,
                  Item_field *outer_field_arg)
     :Item_direct_ref(context_arg, 0, outer_field_arg->table_name,
                      outer_field_arg->field_name),
     outer_ref(outer_field_arg), in_sum_func(0),
-    found_in_select_list(0)
+    found_in_select_list(0), found_in_group_by(0)
   {
     ref= &outer_ref;
     set_properties();
@@ -2444,7 +2450,7 @@ public:
                  bool alias_name_used_arg)
     :Item_direct_ref(context_arg, item, table_name_arg, field_name_arg,
                      alias_name_used_arg),
-    outer_ref(0), in_sum_func(0), found_in_select_list(1)
+    outer_ref(0), in_sum_func(0), found_in_select_list(1), found_in_group_by(0)
   {}
   void save_in_result_field(bool no_conversions)
   {
@@ -2456,6 +2462,7 @@ public:
     return (*ref)->const_item() ? 0 : OUTER_REF_TABLE_BIT;
   }
   virtual Ref_Type ref_type() { return OUTER_REF; }
+  bool check_inner_refs_processor(uchar * arg); 
 };
 
 

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2010-05-09 19:30:06 +0000
+++ b/sql/mysql_priv.h	2010-05-18 04:03:53 +0000
@@ -652,7 +652,8 @@ enum enum_parsing_place
   IN_HAVING,
   SELECT_LIST,
   IN_WHERE,
-  IN_ON
+  IN_ON,
+  IN_GROUP_BY
 };
 
 struct st_table;

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2010-05-10 14:23:08 +0000
+++ b/sql/sql_select.cc	2010-05-18 04:03:53 +0000
@@ -315,6 +315,8 @@ bool handle_select(THD *thd, LEX *lex, s
       function is aggregated in the select where the outer field was
       resolved or in some more inner select then the Item_direct_ref
       class should be used.
+      It used used also if we are grouping by a subquery that refers
+      this outer field.
     The resolution is done here and not at the fix_fields() stage as
     it can be done only after sum functions are fixed and pulled up to
     selects where they are have to be aggregated.
@@ -337,7 +339,19 @@ fix_inner_refs(THD *thd, List<Item> &all
   bool res= FALSE;
   bool direct_ref= FALSE;
 
-  List_iterator<Item_outer_ref> ref_it(select->inner_refs_list);
+  /*
+    Mark the references from  the inner_refs_list that are occurred in
+    the group by expressions. Those references will contain direct
+    references to the referred fields. The markers are set in 
+    the found_in_group_by field of the references from the list.
+  */
+  List_iterator_fast <Item_outer_ref> ref_it(select->inner_refs_list);
+  for (ORDER *group= select->join->group_list; group;  group= group->next)
+  {
+    (*group->item)->walk(&Item::check_inner_refs_processor,
+                         TRUE, (uchar *) &ref_it);
+  } 
+    
   while ((ref= ref_it++))
   {
     Item *item= ref->outer_ref;
@@ -381,6 +395,9 @@ fix_inner_refs(THD *thd, List<Item> &all
         }
       }
     }
+    else if (ref->found_in_group_by)
+      direct_ref= TRUE;
+
     new_ref= direct_ref ?
               new Item_direct_ref(ref->context, item_ref, ref->table_name,
                           ref->field_name, ref->alias_name_used) :
@@ -14908,6 +14925,8 @@ setup_group(THD *thd, Item **ref_pointer
   uint org_fields=all_fields.elements;
 
   thd->where="group statement";
+  enum_parsing_place save_place= thd->lex->current_select->parsing_place;
+  thd->lex->current_select->parsing_place= IN_GROUP_BY;
   for (ord= order; ord; ord= ord->next)
   {
     if (find_order_in_list(thd, ref_pointer_array, tables, ord, fields,
@@ -14920,6 +14939,8 @@ setup_group(THD *thd, Item **ref_pointer
       return 1;
     }
   }
+  thd->lex->current_select->parsing_place= save_place;
+
   if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY)
   {
     /*

    

Igor Babaev

tags

participants (1)