MDEV-4289 Assertion `0' fails in make_sortkey with GROUP_CONCAT, MAKE_SET, GROUP BY

Item_func_make_set wasn't taking into account the first argument when
calculating maybe_null.

sql/item_strfunc.cc:
  rewrite Item_func_make_set, removing separate storage of the first argument
sql/item_strfunc.h:
  rewrite Item_func_make_set, removing separate storage of the first argument

5 files changed, 24 insertions, 85 deletions

diff --git a/mysql-test/r/func_str.result b/mysql-test/r/func_str.result
index aef452b7b50..db2c3430784 100644
--- a/mysql-test/r/func_str.result
+++ b/mysql-test/r/func_str.result
@@ -2646,4 +2646,11 @@ NULL
 SELECT LPAD('hi', DAY(FROM_UNIXTIME(-1)),'?');
 LPAD('hi', DAY(FROM_UNIXTIME(-1)),'?')
 NULL
+create table t1 (i int);
+insert into t1 values (null),(8);
+select group_concat( i ), make_set( i, 'a', 'b' ) field from t1 group by field;
+group_concat( i )	field
+NULL	NULL
+8	
+drop table t1;
 End of 5.1 tests
diff --git a/mysql-test/t/func_str.test b/mysql-test/t/func_str.test
index 9909974d3be..f49fa633925 100644
--- a/mysql-test/t/func_str.test
+++ b/mysql-test/t/func_str.test
@@ -1390,5 +1390,12 @@ SELECT REPEAT('1', DAY(FROM_UNIXTIME(-1)));
 SELECT RPAD('hi', DAY(FROM_UNIXTIME(-1)),'?');
 SELECT LPAD('hi', DAY(FROM_UNIXTIME(-1)),'?');
 
+#
+# MDEV-4289 Assertion `0' fails in make_sortkey with GROUP_CONCAT, MAKE_SET, GROUP BY
+#
+create table t1 (i int);
+insert into t1 values (null),(8);
+select group_concat( i ), make_set( i, 'a', 'b' ) field from t1 group by field;
+drop table t1;
 
 --echo End of 5.1 tests
diff --git a/sql/item_create.cc b/sql/item_create.cc
index a5dc3eeb5ad..78b50ff01fc 100644
--- a/sql/item_create.cc
+++ b/sql/item_create.cc
@@ -3952,8 +3952,7 @@ Create_func_make_set::create_native(THD *thd, LEX_STRING name,
     return NULL;
   }
 
-  Item *param_1= item_list->pop();
-  return new (thd->mem_root) Item_func_make_set(param_1, *item_list);
+  return new (thd->mem_root) Item_func_make_set(*item_list);
 }
 
 
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index 9cc77849094..ffa227834a9 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -2233,37 +2233,14 @@ String *Item_func_elt::val_str(String *str)
 }
 
 
-void Item_func_make_set::split_sum_func(THD *thd, Item **ref_pointer_array,
-					List<Item> &fields)
-{
-  item->split_sum_func2(thd, ref_pointer_array, fields, &item, TRUE);
-  Item_str_func::split_sum_func(thd, ref_pointer_array, fields);
-}
-
-
 void Item_func_make_set::fix_length_and_dec()
 {
-  max_length=arg_count-1;
-
-  if (agg_arg_charsets(collation, args, arg_count, MY_COLL_ALLOW_CONV, 1))
+  if (agg_arg_charsets(collation, args+1, arg_count-1, MY_COLL_ALLOW_CONV, 1))
     return;
   
-  for (uint i=0 ; i < arg_count ; i++)
+  max_length=arg_count-2;
+  for (uint i=1 ; i < arg_count ; i++)
     max_length+=args[i]->max_length;
-
-  used_tables_cache|=	  item->used_tables();
-  not_null_tables_cache&= item->not_null_tables();
-  const_item_cache&=	  item->const_item();
-  with_sum_func= with_sum_func || item->with_sum_func;
-}
-
-
-void Item_func_make_set::update_used_tables()
-{
-  Item_func::update_used_tables();
-  item->update_used_tables();
-  used_tables_cache|=item->used_tables();
-  const_item_cache&=item->const_item();
 }
 
 
@@ -2272,15 +2249,15 @@ String *Item_func_make_set::val_str(String *str)
   DBUG_ASSERT(fixed == 1);
   ulonglong bits;
   bool first_found=0;
-  Item **ptr=args;
+  Item **ptr=args+1;
   String *result=&my_empty_string;
 
-  bits=item->val_int();
-  if ((null_value=item->null_value))
+  bits=args[0]->val_int();
+  if ((null_value=args[0]->null_value))
     return NULL;
 
-  if (arg_count < 64)
-    bits &= ((ulonglong) 1 << arg_count)-1;
+  if (arg_count < 65)
+    bits &= ((ulonglong) 1 << (arg_count-1))-1;
 
   for (; bits; bits >>= 1, ptr++)
   {
@@ -2320,39 +2297,6 @@ String *Item_func_make_set::val_str(String *str)
 }
 
 
-Item *Item_func_make_set::transform(Item_transformer transformer, uchar *arg)
-{
-  DBUG_ASSERT(!current_thd->is_stmt_prepare());
-
-  Item *new_item= item->transform(transformer, arg);
-  if (!new_item)
-    return 0;
-
-  /*
-    THD::change_item_tree() should be called only if the tree was
-    really transformed, i.e. when a new item has been created.
-    Otherwise we'll be allocating a lot of unnecessary memory for
-    change records at each execution.
-  */
-  if (item != new_item)
-    current_thd->change_item_tree(&item, new_item);
-  return Item_str_func::transform(transformer, arg);
-}
-
-
-void Item_func_make_set::print(String *str, enum_query_type query_type)
-{
-  str->append(STRING_WITH_LEN("make_set("));
-  item->print(str, query_type);
-  if (arg_count)
-  {
-    str->append(',');
-    print_args(str, 0, query_type);
-  }
-  str->append(')');
-}
-
-
 String *Item_func_char::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
diff --git a/sql/item_strfunc.h b/sql/item_strfunc.h
index e8fa041af4f..a4fae7c69a1 100644
--- a/sql/item_strfunc.h
+++ b/sql/item_strfunc.h
@@ -497,31 +497,13 @@ public:
 
 class Item_func_make_set :public Item_str_func
 {
-  Item *item;
   String tmp_str;
 
 public:
-  Item_func_make_set(Item *a,List<Item> &list) :Item_str_func(list),item(a) {}
+  Item_func_make_set(List<Item> &list) :Item_str_func(list) {}
   String *val_str(String *str);
-  bool fix_fields(THD *thd, Item **ref)
-  {
-    DBUG_ASSERT(fixed == 0);
-    return ((!item->fixed && item->fix_fields(thd, &item)) ||
-	    item->check_cols(1) ||
-	    Item_func::fix_fields(thd, ref));
-  }
-  void split_sum_func(THD *thd, Item **ref_pointer_array, List<Item> &fields);
   void fix_length_and_dec();
-  void update_used_tables();
   const char *func_name() const { return "make_set"; }
-
-  bool walk(Item_processor processor, bool walk_subquery, uchar *arg)
-  {
-    return item->walk(processor, walk_subquery, arg) ||
-      Item_str_func::walk(processor, walk_subquery, arg);
-  }
-  Item *transform(Item_transformer transformer, uchar *arg);
-  virtual void print(String *str, enum_query_type query_type);
 };