Fix LP BUG#715738

Analysis:
A query with implicit grouping is one with aggregate functions and
no GROUP BY clause. MariaDB inherits from MySQL an SQL extenstion
that allows mixing aggregate functions with non-aggregate fields.
If a query with such mixed select clause produces an empty result
set, the meaning of aggregate functions is well defined - either
NULL (MIN, MAX, etc.), or 0 (count(*)). However the non-aggregated
fields must also have some value, and the only reasonable value in
the case of empty result is NULL.

The cause of the many wrong results was that if a field is declared
as non-nullable (e.g. because it is a PK or NOT NULL), the semantic
analysis and the optimization phases treat this field as non-nullable,
and generate all related query plan elements based on this assumption.

Later during execution, these incorrectly configured/generated query
plan elements result in a wrong result because the selected fields
are not null due to the not-null assumption during optimization.

Solution:
Detect before the context analysys phase that a query uses implicit
grouping with mixed aggregates/non-aggregates, and set all fields
as nullable. The parser already walks the SELECT clause, and
already sets Item::with_sum_func for Items that reference aggreagate
functions. The patch adds a symmetric Item::with_field so that all
Items that reference an Item_field are marked during their
construction at parse time in the same way as with aggregate function
use.

9 files changed, 73 insertions, 5 deletions

diff --git a/sql/item.cc b/sql/item.cc
index 13b6484c4ce..45245f07258 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -382,7 +382,7 @@ Item::Item():
   collation(&my_charset_bin, DERIVATION_COERCIBLE)
 {
   marker= 0;
-  maybe_null=null_value=with_sum_func=unsigned_flag=0;
+  maybe_null=null_value=with_sum_func=with_field=unsigned_flag=0;
   in_rollup= 0;
   decimals= 0; max_length= 0;
   with_subselect= 0;
@@ -429,6 +429,7 @@ Item::Item(THD *thd, Item *item):
   null_value(item->null_value),
   unsigned_flag(item->unsigned_flag),
   with_sum_func(item->with_sum_func),
+  with_field(item->with_field),
   fixed(item->fixed),
   is_autogenerated_name(item->is_autogenerated_name),
   with_subselect(item->with_subselect),
@@ -1890,6 +1891,7 @@ Item_field::Item_field(Field *f)
     if this item is to be reused
   */
   orig_table_name= orig_field_name= "";
+  with_field= 1;
 }
 
 
@@ -1938,6 +1940,7 @@ Item_field::Item_field(THD *thd, Name_resolution_context *context_arg,
     name= (char*) orig_field_name;
   }
   set_field(f);
+  with_field= 1;
 }
 
 
@@ -1952,6 +1955,7 @@ Item_field::Item_field(Name_resolution_context *context_arg,
   collation.set(DERIVATION_IMPLICIT);
   if (select && select->parsing_place != IN_HAVING)
       select->select_n_where_fields++;
+  with_field= 1;
 }
 
 /**
@@ -1968,6 +1972,7 @@ Item_field::Item_field(THD *thd, Item_field *item)
    any_privileges(item->any_privileges)
 {
   collation.set(DERIVATION_IMPLICIT);
+  with_field= 1;
 }
 
 void Item_field::set_field(Field *field_par)
@@ -6285,6 +6290,7 @@ void Item_ref::set_properties()
     split_sum_func() doesn't try to change the reference.
   */
   with_sum_func= (*ref)->with_sum_func;
+  with_field= (*ref)->with_field;
   unsigned_flag= (*ref)->unsigned_flag;
   fixed= 1;
   if (alias_name_used)
@@ -6709,6 +6715,7 @@ Item_cache_wrapper::Item_cache_wrapper(Item *item_arg)
   decimals=   orig_item->decimals;
   collation.set(orig_item->collation);
   with_sum_func= orig_item->with_sum_func;
+  with_field= orig_item->with_field;
   unsigned_flag= orig_item->unsigned_flag;
   name= item_arg->name;
   name_length= item_arg->name_length;
diff --git a/sql/item.h b/sql/item.h
index adbf7103de1..748a5258b1b 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -544,7 +544,11 @@ public:
                                            of a query with ROLLUP */ 
   bool null_value;			/* if item is null */
   bool unsigned_flag;
-  bool with_sum_func;
+  bool with_sum_func;                   /* True if item contains a sum func */
+  /**
+    True if any item except Item_sum_func contains a field. Set during parsing.
+  */
+  bool with_field;
   bool fixed;                           /* If item fixed with fix_fields */
   bool is_autogenerated_name;           /* indicate was name of this Item
                                            autogenerated or set by user */
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 86e81c386e0..af0138402a9 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -1741,6 +1741,7 @@ bool Item_in_optimizer::fix_left(THD *thd, Item **ref)
   }
   not_null_tables_cache= args[0]->not_null_tables();
   with_sum_func= args[0]->with_sum_func;
+  with_field= args[0]->with_field;
   if ((const_item_cache= args[0]->const_item()))
     cache->store(args[0]);
   return 0;
@@ -1766,6 +1767,7 @@ bool Item_in_optimizer::fix_fields(THD *thd, Item **ref)
   if (args[1]->maybe_null)
     maybe_null=1;
   with_sum_func= with_sum_func || args[1]->with_sum_func;
+  with_field= with_field || args[1]->with_field;
   used_tables_cache|= args[1]->used_tables();
   not_null_tables_cache|= args[1]->not_null_tables();
   const_item_cache&= args[1]->const_item();
@@ -2228,6 +2230,7 @@ void Item_func_interval::fix_length_and_dec()
   used_tables_cache|= row->used_tables();
   not_null_tables_cache= row->not_null_tables();
   with_sum_func= with_sum_func || row->with_sum_func;
+  with_field= with_field || row->with_field;
   const_item_cache&= row->const_item();
 }
 
@@ -4380,6 +4383,7 @@ Item_cond::fix_fields(THD *thd, Item **ref)
       const_item_cache= FALSE;
     }  
     with_sum_func=	    with_sum_func || item->with_sum_func;
+    with_field=             with_field || item->with_field;
     with_subselect|=        item->with_subselect;
     if (item->maybe_null)
       maybe_null=1;
@@ -5035,6 +5039,7 @@ Item_func_regex::fix_fields(THD *thd, Item **ref)
        args[1]->fix_fields(thd, args + 1)) || args[1]->check_cols(1))
     return TRUE;				/* purecov: inspected */
   with_sum_func=args[0]->with_sum_func || args[1]->with_sum_func;
+  with_field= args[0]->with_field || args[1]->with_field;
   max_length= 1;
   decimals= 0;
 
diff --git a/sql/item_func.cc b/sql/item_func.cc
index 1823f4bb7a5..63b8419aaaa 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -79,6 +79,7 @@ void Item_func::set_arguments(List<Item> &list)
     {
       *(save_args++)= item;
       with_sum_func|=item->with_sum_func;
+      with_field|= item->with_field;
     }
   }
   list.empty();					// Fields are used
@@ -129,6 +130,7 @@ Item_func::Item_func(THD *thd, Item_func *item)
     Sets as a side effect the following class variables:
       maybe_null	Set if any argument may return NULL
       with_sum_func	Set if any of the arguments contains a sum function
+      with_field        Set if any of the arguments contains or is a field
       used_tables_cache Set to union of the tables used by arguments
 
       str_value.charset If this is a string function, set this to the
@@ -198,6 +200,7 @@ Item_func::fix_fields(THD *thd, Item **ref)
 	maybe_null=1;
 
       with_sum_func= with_sum_func || item->with_sum_func;
+      with_field= with_field || item->with_field;
       used_tables_cache|=     item->used_tables();
       not_null_tables_cache|= item->not_null_tables();
       const_item_cache&=      item->const_item();
@@ -2939,6 +2942,7 @@ udf_handler::fix_fields(THD *thd, Item_result_field *func,
       if (item->maybe_null)
 	func->maybe_null=1;
       func->with_sum_func= func->with_sum_func || item->with_sum_func;
+      func->with_field= func->with_field || item->with_field;
       used_tables_cache|=item->used_tables();
       const_item_cache&=item->const_item();
       f_args.arg_type[i]=item->result_type();
diff --git a/sql/item_func.h b/sql/item_func.h
index 36f089de529..5c5ea33f247 100644
--- a/sql/item_func.h
+++ b/sql/item_func.h
@@ -64,6 +64,7 @@ public:
     allowed_arg_cols(1), arg_count(0)
   {
     with_sum_func= 0;
+    with_field= 0;
   }
   Item_func(Item *a):
     allowed_arg_cols(1), arg_count(1)
@@ -71,6 +72,7 @@ public:
     args= tmp_arg;
     args[0]= a;
     with_sum_func= a->with_sum_func;
+    with_field= a->with_field;
   }
   Item_func(Item *a,Item *b):
     allowed_arg_cols(1), arg_count(2)
@@ -78,6 +80,7 @@ public:
     args= tmp_arg;
     args[0]= a; args[1]= b;
     with_sum_func= a->with_sum_func || b->with_sum_func;
+    with_field= a->with_field || b->with_field;
   }
   Item_func(Item *a,Item *b,Item *c):
     allowed_arg_cols(1)
@@ -88,6 +91,7 @@ public:
       arg_count= 3;
       args[0]= a; args[1]= b; args[2]= c;
       with_sum_func= a->with_sum_func || b->with_sum_func || c->with_sum_func;
+      with_field= a->with_field || b->with_field || c->with_field;
     }
   }
   Item_func(Item *a,Item *b,Item *c,Item *d):
@@ -100,6 +104,8 @@ public:
       args[0]= a; args[1]= b; args[2]= c; args[3]= d;
       with_sum_func= a->with_sum_func || b->with_sum_func ||
 	c->with_sum_func || d->with_sum_func;
+      with_field= a->with_field || b->with_field ||
+        c->with_field || d->with_field;
     }
   }
   Item_func(Item *a,Item *b,Item *c,Item *d,Item* e):
@@ -111,6 +117,8 @@ public:
       args[0]= a; args[1]= b; args[2]= c; args[3]= d; args[4]= e;
       with_sum_func= a->with_sum_func || b->with_sum_func ||
 	c->with_sum_func || d->with_sum_func || e->with_sum_func ;
+      with_field= a->with_field || b->with_field ||
+        c->with_field || d->with_field || e->with_field;
     }
   }
   Item_func(List<Item> &list);
diff --git a/sql/item_row.cc b/sql/item_row.cc
index 22421199b76..99a1644cc48 100644
--- a/sql/item_row.cc
+++ b/sql/item_row.cc
@@ -86,6 +86,7 @@ bool Item_row::fix_fields(THD *thd, Item **ref)
     }
     maybe_null|= item->maybe_null;
     with_sum_func= with_sum_func || item->with_sum_func;
+    with_field= with_field || item->with_field;
   }
   fixed= 1;
   return FALSE;
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index ad88fe31a0d..39776643ddd 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -2248,6 +2248,7 @@ void Item_func_make_set::fix_length_and_dec()
   not_null_tables_cache&= item->not_null_tables();
   const_item_cache&=	  item->const_item();
   with_sum_func= with_sum_func || item->with_sum_func;
+  with_field= with_field || item->with_field;
 }
 
 
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index cc2704efb32..7de02d726fa 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -415,6 +415,7 @@ void Item_sum::mark_as_sum_func()
   cur_select->n_sum_items++;
   cur_select->with_sum_func= 1;
   with_sum_func= 1;
+  with_field= 0;
 }
 
 
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 7bd09cb831d..2ade04e8592 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -517,12 +517,49 @@ JOIN::prepare(Item ***rref_pointer_array,
                                     tables_list, &select_lex->leaf_tables,
                                     FALSE, SELECT_ACL, SELECT_ACL))
       DBUG_RETURN(-1);
+  /*
+    TRUE if the SELECT list mixes elements with and without grouping,
+    and there is no GROUP BY clause. Mixing non-aggregated fields with
+    aggregate functions in the SELECT list is a MySQL exptenstion that
+    is allowed only if the ONLY_FULL_GROUP_BY sql mode is not set.
+  */
+  bool mixed_implicit_grouping= false;
+  if ((~thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY) &&
+      select_lex->with_sum_func && !group_list)
+  {
+    List_iterator_fast <Item> select_it(fields_list);
+    Item *select_el; /* Element of the SELECT clause, can be an expression. */
+    bool found_field_elem= false;
+    bool found_sum_func_elem= false;
+
+    while ((select_el= select_it++))
+    {
+      if (select_el->with_sum_func)
+        found_sum_func_elem= true;
+      if (select_el->with_field)
+        found_field_elem= true;
+      if (found_sum_func_elem && found_field_elem)
+      {
+        mixed_implicit_grouping= true;
+        break;
+      }
+    }
+  }
  
-  TABLE_LIST *table_ptr;
-  for (table_ptr= select_lex->leaf_tables;
+  for (TABLE_LIST *table_ptr= select_lex->leaf_tables;
        table_ptr;
        table_ptr= table_ptr->next_leaf)
-    tables++;
+  {
+    tables++; /* Count the number of tables in the join. */
+    /*
+      If the query uses implicit grouping where the select list contains both
+      aggregate functions and non-aggregate fields, any non-aggregated field
+      may produce a NULL value. Set all fields of each table as nullable before
+      semantic analysis to take into account this change of nullability.
+    */
+    if (mixed_implicit_grouping)
+      table_ptr->table->maybe_null= 1;
+  }
 
   if (setup_wild(thd, tables_list, fields_list, &all_fields, wild_num) ||
       select_lex->setup_ref_array(thd, og_num) ||