Fixes done while working on MDEV-4119:

Fixed several optimizer issues relatied to GROUP BY:

a) Refering to a SELECT column in HAVING sometimes calculated it twice, which caused problems with non determinstic functions

b) Removing duplicate fields and constants from GROUP BY was done too late for "using index for group by" optimization to work

c) EXPLAIN SELECT ... GROUP BY did wrongly show 'Using filesort' in some cases involving "Using index for group-by"


a) was fixed by:
- Changed last argument to Item::split_sum_func2() from bool to int to allow more flags
- Added flag argument to Item::split_sum_func() to allow on to specify if the item was in the SELECT part
- Mark all split_sum_func() calls from SELECT with SPLIT_SUM_SELECT
- Changed split_sum_func2() to do nothing if called with an argument that is not a sum function and doesn't include sum functions, if we are not an argument to SELECT.

This ensures that in a case like
select a*sum(b) as f1 from t1 where a=1 group by c having f1 <= 10;

That 'a' in the SELECT part is stored as a reference in the temporary table togeher with sum(b) while the 'a' in having isn't (not needed as 'a' is already a reference to a column in the result)

b) was fixed by:
- Added an extra remove_const() pass for GROUP BY arguments before make_join_statistics() in case of one table SELECT.

This allowes get_best_group_min_max() to optimize things better.

c) was fixed by:
- Added test for group by optimization in JOIN::exec_inner for
  select->quick->get_type() == QUICK_SELECT_I::QS_TYPE_GROUP_MIN_MAX

item.cc:
- Simplifed Item::split_sum_func2()
  - Split test to make them faster and easier to read
  - Changed last argument to Item::split_sum_func2() from bool to int to allow more flags
  - Added flag argument to Item::split_sum_func() to allow on to specify if the item was in the SELECT part
  - Changed split_sum_func2() to do nothing if called with an argument that is not a sum function and doesn't include sum functions, if we are not an argument to SELECT.

opt_range.cc:
- Simplified get_best_group_min_max() by calcuating first how many group_by elements.
- Use join->group instead of join->group_list to test if group by, as join->group_list may be NULL if everything was optimized away.

sql_select.cc:
- Added an extra remove_const() pass for GROUP BY arguments before make_join_statistics() in case of one table SELECT.
- Use group instead of group_list to test if group by, as group_list may be NULL if everything was optimized away.
- Moved printing of "Error in remove_const" to remove_const() instead of having it in caller.
- Simplified some if tests by re-ordering code.
- update_depend_map_for_order() and remove_const() fixed to handle the case where make_join_statistics() has not yet been called (join->join_tab is 0 in this case)

Author: montywi

mysql-test/r/group_by.result

@@ -522,6 +522,7 @@ NULL	9
 	3
 b	1
 drop table t1;
+set big_tables=0;
 create table t1 (a int not null, b int not null);
 insert into t1 values (1,1),(1,2),(3,1),(3,2),(2,2),(2,1);
 create table t2 (a int not null, b int not null, key(a));
@@ -659,7 +660,10 @@ insert into t1 (a,b) values (1,2),(1,3),(2,5);
 select a, 0.1*0+1 r2, sum(1) r1 from t1 where a = 1 group  by a having r1>1 and r2=1;
 a	r2	r1
 1	1.0	2
-select a, round(rand(100)*10) r2, sum(1) r1 from t1 where a = 1 group  by a having r1>1 and r2<=2;
+select a, round(rand(100)*10) r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2<=2;
+a	r2	r1
+1	2	2
+select a, round(rand(100)*10) r2, sum(1) r1 from t1  group by a having r1>1 and r2<=2;
 a	r2	r1
 1	2	2
 select a,sum(b) from t1 where a=1 group by c;
@@ -668,6 +672,12 @@ a	sum(b)
 select a*sum(b) from t1 where a=1 group by c;
 a*sum(b)
 5
+select a*sum(b) as f1 from t1 where a=1 group by c having f1 <= 10;
+f1
+5
+select a,a*sum(b) as f1 from t1 where a=1 group by c having a*sum(b)+0 <= 10;
+a	f1
+1	5
 select sum(a)*sum(b) from t1 where a=1 group by c;
 sum(a)*sum(b)
 10
@@ -1988,12 +1998,12 @@ SHOW SESSION STATUS LIKE 'Sort_scan%';
 Variable_name	Value
 Sort_scan	0
 EXPLAIN SELECT SQL_BIG_RESULT col1 AS field1, col1 AS field2
-FROM t1 GROUP BY field1, field2;;
+FROM t1 GROUP BY field1, field2;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index; Using filesort
 FLUSH STATUS;
 SELECT SQL_BIG_RESULT col1 AS field1, col1 AS field2
-FROM t1 GROUP BY field1, field2;;
+FROM t1 GROUP BY field1, field2;
 field1	field2
 1	1
 2	2
@@ -2082,6 +2092,58 @@ f1	f2
 19	19
 20	20
 explain
+select col1 f1, col1 f2 from t1 group by f1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index
+select col1 f1, col1 f2 from t1 group by f1;
+f1	f2
+1	1
+2	2
+3	3
+4	4
+5	5
+6	6
+7	7
+8	8
+9	9
+10	10
+11	11
+12	12
+13	13
+14	14
+15	15
+16	16
+17	17
+18	18
+19	19
+20	20
+explain
+select col1 f1, col1 f2 from t1 group by f1, f2;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index
+select col1 f1, col1 f2 from t1 group by f1, f2;
+f1	f2
+1	1
+2	2
+3	3
+4	4
+5	5
+6	6
+7	7
+8	8
+9	9
+10	10
+11	11
+12	12
+13	13
+14	14
+15	15
+16	16
+17	17
+18	18
+19	19
+20	20
+explain
 select col1 f1, col1 f2 from t1 group by f2 order by f2, f1;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index
@@ -2141,6 +2203,22 @@ INSERT INTO t2(col1, col2) VALUES
 (1,20),(2,19),(3,18),(4,17),(5,16),(6,15),(7,14),(8,13),(9,12),(10,11),
 (11,10),(12,9),(13,8),(14,7),(15,6),(16,5),(17,4),(18,3),(19,2),(20,1);
 explain
+select col1 f1, col2 f2, col1 f3 from t2 group by f1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	range	NULL	idx	5	NULL	7	Using index for group-by
+explain
+select SQL_BIG_RESULT col1 f1, col2 f2, col1 f3 from t2 group by f1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	range	NULL	idx	5	NULL	7	Using index for group-by
+explain
+select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	index	NULL	idx	10	NULL	20	Using index
+explain
+select col1 f1, col1 f2 from t2 group by f1, 1+1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	range	NULL	idx	5	NULL	7	Using index for group-by
+explain
 select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3+0;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t2	index	NULL	idx	10	NULL	20	Using index; Using temporary; Using filesort
@@ -2167,6 +2245,10 @@ f1	f2	f3
 19	2	19
 20	1	20
 explain
+select col1 f1, col2 f2, col1 f3 from t2 order by f1,f2;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	index	NULL	idx	10	NULL	20	Using index
+explain
 select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3+0;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t2	index	NULL	idx	10	NULL	20	Using index; Using filesort

mysql-test/t/group_by.test

@@ -394,6 +394,7 @@ select a,count(*) from t1 group by a;
 set big_tables=1;
 select a,count(*) from t1 group by a;
 drop table t1;
+set big_tables=0;
 
 #
 # Test of GROUP BY ... ORDER BY NULL optimization
@@ -485,9 +486,12 @@ create table t1 (a integer, b integer, c integer);
 insert into t1 (a,b) values (1,2),(1,3),(2,5);
 select a, 0.1*0+1 r2, sum(1) r1 from t1 where a = 1 group  by a having r1>1 and r2=1;
 # rand(100)*10 will be < 2 only for the first row (of 6)
-select a, round(rand(100)*10) r2, sum(1) r1 from t1 where a = 1 group  by a having r1>1 and r2<=2;
+select a, round(rand(100)*10) r2, sum(1) r1 from t1 where a = 1 group by a having r1>1 and r2<=2;
+select a, round(rand(100)*10) r2, sum(1) r1 from t1  group by a having r1>1 and r2<=2;
 select a,sum(b) from t1 where a=1 group by c;
 select a*sum(b) from t1 where a=1 group by c;
+select a*sum(b) as f1 from t1 where a=1 group by c having f1 <= 10;
+select a,a*sum(b) as f1 from t1 where a=1 group by c having a*sum(b)+0 <= 10;
 select sum(a)*sum(b) from t1 where a=1 group by c;
 select a,sum(b) from t1 where a=1 group by c having a=1;
 select a as d,sum(b) from t1 where a=1 group by c having d=1;
@@ -1346,9 +1350,9 @@ let $query=SELECT SQL_BIG_RESULT col1 AS field1, col1 AS field2
            FROM t1 GROUP BY field1, field2;
 
 # Needs to be range to exercise bug
---eval EXPLAIN $query;
+--eval EXPLAIN $query
 FLUSH STATUS;
---eval $query;
+--eval $query
 SHOW SESSION STATUS LIKE 'Sort_scan%';
 
 CREATE VIEW v1 AS SELECT * FROM t1;
@@ -1368,6 +1372,14 @@ explain
 select col1 f1, col1 f2 from t1 order by f2, f1+0;
 select col1 f1, col1 f2 from t1 order by f2, f1+0;
 
+explain
+select col1 f1, col1 f2 from t1 group by f1;
+select col1 f1, col1 f2 from t1 group by f1;
+
+explain
+select col1 f1, col1 f2 from t1 group by f1, f2;
+select col1 f1, col1 f2 from t1 group by f1, f2;
+
 explain
 select col1 f1, col1 f2 from t1 group by f2 order by f2, f1;
 select col1 f1, col1 f2 from t1 group by f2 order by f2, f1;
@@ -1376,6 +1388,7 @@ explain
 select col1 f1, col1 f2 from t1 group by f1, f2 order by f2, f1;
 select col1 f1, col1 f2 from t1 group by f1, f2 order by f2, f1;
 
+
 CREATE TABLE t2(
  col1 int, 
  col2 int, 
@@ -1385,10 +1398,21 @@ INSERT INTO t2(col1, col2) VALUES
    (1,20),(2,19),(3,18),(4,17),(5,16),(6,15),(7,14),(8,13),(9,12),(10,11),
    (11,10),(12,9),(13,8),(14,7),(15,6),(16,5),(17,4),(18,3),(19,2),(20,1);
 
+explain
+select col1 f1, col2 f2, col1 f3 from t2 group by f1;
+explain
+select SQL_BIG_RESULT col1 f1, col2 f2, col1 f3 from t2 group by f1;
+explain
+select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2;
+explain
+select col1 f1, col1 f2 from t2 group by f1, 1+1;
+
 explain
 select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3+0;
 select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3+0;
 
+explain
+select col1 f1, col2 f2, col1 f3 from t2 order by f1,f2;
 explain
 select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3+0;
 select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3+0;

sql/item.cc

@@ -1705,69 +1705,98 @@ class Item_aggregate_ref : public Item_ref
 
   @param thd			Thread handler
   @param ref_pointer_array	Pointer to array of reference fields
-  @param fields		All fields in select
+  @param fields		        All fields in select
   @param ref			Pointer to item
-  @param skip_registered       <=> function be must skipped for registered
-                               SUM items
+  @param split_flags            Zero or more of the following flags
+	                        SPLIT_FUNC_SKIP_REGISTERED:
+                                Function be must skipped for registered SUM
+                                SUM items
+                                SPLIT_FUNC_SELECT
+                                We are called on the select level and have to
+                                register items operated on sum function
 
   @note
-    This is from split_sum_func2() for items that should be split
-
     All found SUM items are added FIRST in the fields list and
     we replace the item with a reference.
 
+    If this is an item in the SELECT list then we also have to split out
+    all arguments to functions used together with the sum function.
+    For example in case of SELECT A*sum(B) we have to split out both
+    A and sum(B).
+    This is not needed for ORDER BY, GROUP BY or HAVING as all references
+    to items in the select list are already of type REF
+
     thd->fatal_error() may be called if we are out of memory
 */
 
 void Item::split_sum_func2(THD *thd, Item **ref_pointer_array,
                            List<Item> &fields, Item **ref, 
-                           bool skip_registered)
+                           uint split_flags)
 {
-  /* An item of type Item_sum  is registered <=> ref_by != 0 */ 
-  if (type() == SUM_FUNC_ITEM && skip_registered && 
-      ((Item_sum *) this)->ref_by)
-    return;
-  if ((type() != SUM_FUNC_ITEM && with_sum_func) ||
-      (type() == FUNC_ITEM &&
-       (((Item_func *) this)->functype() == Item_func::ISNOTNULLTEST_FUNC ||
-        ((Item_func *) this)->functype() == Item_func::TRIG_COND_FUNC)))
+  if (unlikely(type() == SUM_FUNC_ITEM))
   {
-    /* Will split complicated items and ignore simple ones */
-    split_sum_func(thd, ref_pointer_array, fields);
+    /* An item of type Item_sum is registered if ref_by != 0 */
+    if ((split_flags & SPLIT_SUM_SKIP_REGISTERED) && 
+        ((Item_sum *) this)->ref_by)
+      return;
   }
-  else if ((type() == SUM_FUNC_ITEM || (used_tables() & ~PARAM_TABLE_BIT)) &&
-           type() != SUBSELECT_ITEM &&
-           (type() != REF_ITEM ||
-           ((Item_ref*)this)->ref_type() == Item_ref::VIEW_REF))
+  else
   {
-    /*
-      Replace item with a reference so that we can easily calculate
-      it (in case of sum functions) or copy it (in case of fields)
-
-      The test above is to ensure we don't do a reference for things
-      that are constants (PARAM_TABLE_BIT is in effect a constant)
-      or already referenced (for example an item in HAVING)
-      Exception is Item_direct_view_ref which we need to convert to
-      Item_ref to allow fields from view being stored in tmp table.
-    */
-    Item_aggregate_ref *item_ref;
-    uint el= fields.elements;
-    /*
-      If this is an item_ref, get the original item
-      This is a safety measure if this is called for things that is
-      already a reference.
-    */
-    Item *real_itm= real_item();
+    /* Not a SUM() function */
+    if (unlikely((!with_sum_func && !(split_flags & SPLIT_SUM_SELECT))))
+    {
+      /*
+        This is not a SUM function and there are no SUM functions inside.
+        Nothing more to do.
+      */
+      return;
+    }
+    if (likely(with_sum_func ||
+               (type() == FUNC_ITEM &&
+                (((Item_func *) this)->functype() ==
+                 Item_func::ISNOTNULLTEST_FUNC ||
+                 ((Item_func *) this)->functype() ==
+                 Item_func::TRIG_COND_FUNC))))
+    {
+      /* Will call split_sum_func2() for all items */
+      split_sum_func(thd, ref_pointer_array, fields, split_flags);
+      return;
+    }
 
-    ref_pointer_array[el]= real_itm;
-    if (!(item_ref= new Item_aggregate_ref(&thd->lex->current_select->context,
-                                           ref_pointer_array + el, 0, name)))
-      return;                                   // fatal_error is set
-    if (type() == SUM_FUNC_ITEM)
-      item_ref->depended_from= ((Item_sum *) this)->depended_from(); 
-    fields.push_front(real_itm);
-    thd->change_item_tree(ref, item_ref);
+    if (unlikely((!(used_tables() & ~PARAM_TABLE_BIT) ||
+                  type() == SUBSELECT_ITEM ||
+                  (type() == REF_ITEM &&
+                   ((Item_ref*)this)->ref_type() != Item_ref::VIEW_REF))))
+        return;
   }
+
+  /*
+    Replace item with a reference so that we can easily calculate
+    it (in case of sum functions) or copy it (in case of fields)
+
+    The test above is to ensure we don't do a reference for things
+    that are constants (PARAM_TABLE_BIT is in effect a constant)
+    or already referenced (for example an item in HAVING)
+    Exception is Item_direct_view_ref which we need to convert to
+    Item_ref to allow fields from view being stored in tmp table.
+  */
+  Item_aggregate_ref *item_ref;
+  uint el= fields.elements;
+  /*
+    If this is an item_ref, get the original item
+    This is a safety measure if this is called for things that is
+    already a reference.
+  */
+  Item *real_itm= real_item();
+
+  ref_pointer_array[el]= real_itm;
+  if (!(item_ref= new Item_aggregate_ref(&thd->lex->current_select->context,
+                                         ref_pointer_array + el, 0, name)))
+    return;                                   // fatal_error is set
+  if (type() == SUM_FUNC_ITEM)
+    item_ref->depended_from= ((Item_sum *) this)->depended_from(); 
+  fields.push_front(real_itm);
+  thd->change_item_tree(ref, item_ref);
 }
 
 

sql/item.h

@@ -74,6 +74,10 @@ char_to_byte_length_safe(uint32 char_length_arg, uint32 mbmaxlen_arg)
 }
 
 
+/* Bits for the split_sum_func() function */
+#define SPLIT_SUM_SKIP_REGISTERED 1     /* Skip registered funcs */
+#define SPLIT_SUM_SELECT 2		/* SELECT item; Split all parts */
+
 /*
    "Declared Type Collation"
    A combination of collation and its derivation.
@@ -1169,10 +1173,10 @@ class Item: public Type_std_attributes
     return false;
   }
   virtual void split_sum_func(THD *thd, Item **ref_pointer_array,
-                              List<Item> &fields) {}
+                              List<Item> &fields, uint flags) {}
   /* Called for items that really have to be split */
   void split_sum_func2(THD *thd, Item **ref_pointer_array, List<Item> &fields,
-                       Item **ref, bool skip_registered);
+                       Item **ref, uint flags);
   virtual bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate);
   bool get_time(MYSQL_TIME *ltime)
   { return get_date(ltime, TIME_TIME_ONLY | TIME_INVALID_DATES); }