WIP10.5-mdev22011

For JSON_ARRAYAGG with DISTINCT also store the null bytes for the record
inside the Unique object

4 files changed, 206 insertions, 18 deletions

diff --git a/mysql-test/main/func_json.result b/mysql-test/main/func_json.result
index c87a2998724..4ef60be9d8f 100644
--- a/mysql-test/main/func_json.result
+++ b/mysql-test/main/func_json.result
@@ -1160,13 +1160,13 @@ JSON_ARRAYAGG(DISTINCT a)
 [1,2,3]
 SELECT JSON_ARRAYAGG(DISTINCT b) FROM t1;
 JSON_ARRAYAGG(DISTINCT b)
-["Hello","World","This","Will","Work","!",null]
+[null,"!","Hello","This","Will","Work","World"]
 SELECT JSON_ARRAYAGG(DISTINCT a LIMIT 2) FROM t1;
 JSON_ARRAYAGG(DISTINCT a LIMIT 2)
 [1,2]
 SELECT JSON_ARRAYAGG(DISTINCT b LIMIT 2) FROM t1;
 JSON_ARRAYAGG(DISTINCT b LIMIT 2)
-["Hello","World"]
+[null,"!"]
 #
 # JSON aggregation
 #
diff --git a/sql/item_jsonfunc.cc b/sql/item_jsonfunc.cc
index 442e9b5bdca..7e2dc82cf1f 100644
--- a/sql/item_jsonfunc.cc
+++ b/sql/item_jsonfunc.cc
@@ -1461,7 +1461,7 @@ static int append_json_value_from_field(String *str,
     const char *t_f;
     int t_f_len;
 
-    if (f->is_null(offset))
+    if (f->is_null_in_record(key))
       goto append_null;
 
     if (v_int)
@@ -1479,7 +1479,7 @@ static int append_json_value_from_field(String *str,
   }
   {
     String *sv= f->val_str(tmp_val, key + offset);
-    if (f->is_null(offset))
+    if (f->is_null_in_record(key))
       goto append_null;
     if (i->is_json_type())
       return str->append(sv->ptr(), sv->length());
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index c357df4a63c..519d70ced98 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -3520,7 +3520,7 @@ String *Item_sum_udf_str::val_str(String *str)
 */
 
 extern "C"
-int group_concat_key_cmp_with_distinct(void* arg, const void* key1, 
+int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
                                        const void* key2)
 {
   Item_func_group_concat *item_func= (Item_func_group_concat*)arg;
@@ -3554,12 +3554,68 @@ int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
 }
 
 
+/*
+  @brief
+    Comparator function for DISTINCT clause taking into account NULL values.
+
+  @note
+    Used for JSON_ARRAYAGG function
+*/
+
+int group_concat_key_cmp_with_distinct_with_nulls(void* arg,
+                                                  const void* key1_arg,
+                                                  const void* key2_arg)
+{
+  Item_func_group_concat *item_func= (Item_func_group_concat*)arg;
+
+  uchar *key1= (uchar*)key1_arg + item_func->table->s->null_bytes;
+  uchar *key2= (uchar*)key2_arg + item_func->table->s->null_bytes;
+
+  for (uint i= 0; i < item_func->arg_count_field; i++)
+  {
+    Item *item= item_func->args[i];
+    /*
+      If item is a const item then either get_tmp_table_field returns 0
+      or it is an item over a const table.
+    */
+    if (item->const_item())
+      continue;
+    /*
+      We have to use get_tmp_table_field() instead of
+      real_item()->get_tmp_table_field() because we want the field in
+      the temporary table, not the original field
+    */
+    Field *field= item->get_tmp_table_field();
+
+    if (!field)
+      continue;
+
+    if (field->is_null_in_record((uchar*)key1_arg) &&
+        field->is_null_in_record((uchar*)key2_arg))
+      continue;
+
+    if (field->is_null_in_record((uchar*)key1_arg))
+      return -1;
+
+    if (field->is_null_in_record((uchar*)key2_arg))
+      return 1;
+
+    uint offset= (field->offset(field->table->record[0]) -
+                  field->table->s->null_bytes);
+    int res= field->cmp(key1 + offset, key2 + offset);
+    if (res)
+      return res;
+  }
+  return 0;
+}
+
+
 /**
   function of sort for syntax: GROUP_CONCAT(expr,... ORDER BY col,... )
 */
 
 extern "C"
-int group_concat_key_cmp_with_order(void* arg, const void* key1, 
+int group_concat_key_cmp_with_order(void* arg, const void* key1,
                                     const void* key2)
 {
   Item_func_group_concat* grp_item= (Item_func_group_concat*) arg;
@@ -3577,7 +3633,57 @@ int group_concat_key_cmp_with_order(void* arg, const void* key1,
     if (item->const_item())
       continue;
     /*
-      If item is a const item then either get_tmp_table_field returns 0
+      We have to use get_tmp_table_field() instead of
+      real_item()->get_tmp_table_field() because we want the field in
+      the temporary table, not the original field
+
+      Note that for the case of ROLLUP, field may point to another table
+      tham grp_item->table. This is however ok as the table definitions are
+      the same.
+    */
+    Field *field= item->get_tmp_table_field();
+    if (!field)
+      continue;
+
+    uint offset= (field->offset(field->table->record[0]) -
+                  field->table->s->null_bytes);
+    int res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset);
+    if (res)
+      return ((*order_item)->direction == ORDER::ORDER_ASC) ? res : -res;
+  }
+  /*
+    We can't return 0 because in that case the tree class would remove this
+    item as double value. This would cause problems for case-changes and
+    if the returned values are not the same we do the sort on.
+  */
+  return 1;
+}
+
+
+/*
+  @brief
+    Comparator function for ORDER BY clause taking into account NULL values.
+
+  @note
+    Used for JSON_ARRAYAGG function
+*/
+
+int group_concat_key_cmp_with_order_with_nulls(void *arg, const void *key1_arg,
+                                               const void *key2_arg)
+{
+  Item_func_group_concat* grp_item= (Item_func_group_concat*) arg;
+  ORDER **order_item, **end;
+
+  uchar *key1= (uchar*)key1_arg + grp_item->table->s->null_bytes;
+  uchar *key2= (uchar*)key2_arg + grp_item->table->s->null_bytes;
+
+  for (order_item= grp_item->order, end=order_item+ grp_item->arg_count_order;
+       order_item < end;
+       order_item++)
+  {
+    Item *item= *(*order_item)->item;
+    /*
+      If field_item is a const item then either get_tmp_table_field returns 0
       or it is an item over a const table.
     */
     if (item->const_item())
@@ -3595,6 +3701,16 @@ int group_concat_key_cmp_with_order(void* arg, const void* key1,
     if (!field)
       continue;
 
+    if (field->is_null_in_record((uchar*)key1_arg) &&
+        field->is_null_in_record((uchar*)key2_arg))
+      continue;
+
+    if (field->is_null_in_record((uchar*)key1_arg))
+      return ((*order_item)->direction == ORDER::ORDER_ASC) ?  -1 : 1;
+
+    if (field->is_null_in_record((uchar*)key2_arg))
+      return ((*order_item)->direction == ORDER::ORDER_ASC) ?  1 :  -1;
+
     uint offset= (field->offset(field->table->record[0]) -
                   field->table->s->null_bytes);
     int res= field->cmp((uchar*)key1 + offset, (uchar*)key2 + offset);
@@ -3652,9 +3768,11 @@ int dump_leaf_key(void* key_arg, element_count count __attribute__((unused)),
     result->append(*item->separator);
 
 
+  bool is_null= false;
   for (; arg < arg_end; arg++)
   {
     String *res;
+    is_null= false;
     /*
       We have to use get_tmp_table_field() instead of
       real_item()->get_tmp_table_field() because we want the field in
@@ -3672,12 +3790,12 @@ int dump_leaf_key(void* key_arg, element_count count __attribute__((unused)),
         uint offset= (field->offset(field->table->record[0]) -
                       table->s->null_bytes);
         DBUG_ASSERT(offset < table->s->reclength);
-        res= item->get_str_from_field(*arg, field, &tmp, key, offset);
+        res= item->get_str_from_field(*arg, field, &tmp, key,
+                                      offset + item->get_null_bytes());
       }
       else
         res= item->get_str_from_item(*arg, &tmp);
     }
-
     if (res)
       result->append(*res);
   }
@@ -3957,7 +4075,7 @@ bool Item_func_group_concat::repack_tree(THD *thd)
 
   init_tree(&st.tree, (size_t) MY_MIN(thd->variables.max_heap_table_size,
                                       thd->variables.sortbuff_size/16), 0,
-            size, group_concat_key_cmp_with_order, NULL,
+            size, get_comparator_function_for_order_by(), NULL,
             (void*) this, MYF(MY_THREAD_SPECIFIC));
   DBUG_ASSERT(tree->size_of_element == st.tree.size_of_element);
   st.table= table;
@@ -4012,6 +4130,14 @@ bool Item_func_group_concat::add(bool exclude_nulls)
       if (tree && (res= field->val_str(&buf)))
         row_str_len+= res->length();
     }
+    else
+    {
+      /*
+        should not reach here, we create temp table for all the arguments of
+        the group_concat function
+      */
+      DBUG_ASSERT(0);
+    }
   }
 
   null_value= FALSE;
@@ -4021,7 +4147,7 @@ bool Item_func_group_concat::add(bool exclude_nulls)
   {
     /* Filter out duplicate rows. */
     uint count= unique_filter->elements_in_tree();
-    unique_filter->unique_add(table->record[0] + table->s->null_bytes);
+    unique_filter->unique_add(get_record_pointer());
     if (count == unique_filter->elements_in_tree())
       row_eligible= FALSE;
   }
@@ -4035,8 +4161,7 @@ bool Item_func_group_concat::add(bool exclude_nulls)
         && tree->elements_in_tree > 1)
       if (repack_tree(thd))
         return 1;
-    el= tree_insert(tree, table->record[0] + table->s->null_bytes, 0,
-                    tree->custom_arg);
+    el= tree_insert(tree, get_record_pointer(), 0, tree->custom_arg);
     /* check if there was enough memory to insert the row */
     if (!el)
       return 1;
@@ -4047,7 +4172,7 @@ bool Item_func_group_concat::add(bool exclude_nulls)
     row to the output buffer here. That will be done in val_str.
   */
   if (row_eligible && !warning_for_row && (!tree && !distinct))
-    dump_leaf_key(table->record[0] + table->s->null_bytes, 1, this);
+    dump_leaf_key(get_record_pointer(), 1, this);
 
   return 0;
 }
@@ -4240,16 +4365,16 @@ bool Item_func_group_concat::setup(THD *thd)
     */
     init_tree(tree, (size_t)MY_MIN(thd->variables.max_heap_table_size,
                                    thd->variables.sortbuff_size/16), 0,
-              tree_key_length,
-              group_concat_key_cmp_with_order, NULL, (void*) this,
+              tree_key_length + get_null_bytes(),
+              get_comparator_function_for_order_by(), NULL, (void*) this,
               MYF(MY_THREAD_SPECIFIC));
     tree_len= 0;
   }
 
   if (distinct)
-    unique_filter= new Unique(group_concat_key_cmp_with_distinct,
+    unique_filter= new Unique(get_comparator_function_for_distinct(),
                               (void*)this,
-                              tree_key_length,
+                              tree_key_length + get_null_bytes(),
                               ram_limitation(thd));
   if ((row_limit && row_limit->cmp_type() != INT_RESULT) ||
       (offset_limit && offset_limit->cmp_type() != INT_RESULT))
@@ -4305,6 +4430,55 @@ String* Item_func_group_concat::val_str(String* str)
 }
 
 
+qsort_cmp2 Item_func_group_concat::get_comparator_function_for_distinct()
+{
+  return skip_nulls() ?
+         group_concat_key_cmp_with_distinct :
+         group_concat_key_cmp_with_distinct_with_nulls;
+}
+
+
+qsort_cmp2 Item_func_group_concat::get_comparator_function_for_order_by()
+{
+  return skip_nulls() ?
+         group_concat_key_cmp_with_order :
+         group_concat_key_cmp_with_order_with_nulls;
+}
+
+/*
+
+  @brief
+    Get the record pointer of the current row of the table
+
+  @details
+    look at the comments for Item_func_group_concat::get_null_bytes
+*/
+
+uchar* Item_func_group_concat::get_record_pointer()
+{
+  return skip_nulls() ?
+         table->record[0] + table->s->null_bytes :
+         table->record[0];
+}
+
+
+/*
+  @brief
+    Get the null bytes for the table if required.
+
+  @details
+    This function is used for GROUP_CONCAT (or JSON_ARRAYAGG) implementaton
+    where the Unique tree or the ORDER BY tree may store the null values,
+    in such case we also store the null nyntes inside each node of the tree.
+
+*/
+
+uint Item_func_group_concat::get_null_bytes()
+{
+  return skip_nulls() ? 0 : table->s->null_bytes;
+}
+
+
 void Item_func_group_concat::print(String *str, enum_query_type query_type)
 {
   str->append(func_name());
diff --git a/sql/item_sum.h b/sql/item_sum.h
index 2fb540459f5..0f46b4add73 100644
--- a/sql/item_sum.h
+++ b/sql/item_sum.h
@@ -1847,8 +1847,12 @@ public:
 C_MODE_START
 int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
                                        const void* key2);
+int group_concat_key_cmp_with_distinct_with_nulls(void* arg, const void* key1,
+                                                  const void* key2);
 int group_concat_key_cmp_with_order(void* arg, const void* key1,
                                     const void* key2);
+int group_concat_key_cmp_with_order_with_nulls(void *arg, const void *key1,
+                                               const void *key2);
 int dump_leaf_key(void* key_arg,
                   element_count count __attribute__((unused)),
                   void* item_arg);
@@ -1911,8 +1915,13 @@ protected:
 
   friend int group_concat_key_cmp_with_distinct(void* arg, const void* key1,
                                                 const void* key2);
+  friend int group_concat_key_cmp_with_distinct_with_nulls(void* arg,
+                                                           const void* key1,
+                                                           const void* key2);
   friend int group_concat_key_cmp_with_order(void* arg, const void* key1,
 					     const void* key2);
+  friend int group_concat_key_cmp_with_order_with_nulls(void *arg,
+                                       const void *key1, const void *key2);
   friend int dump_leaf_key(void* key_arg,
                            element_count count __attribute__((unused)),
 			   void* item_arg);
@@ -2002,6 +2011,11 @@ public:
     { context= (Name_resolution_context *)cntx; return FALSE; }
   Item *get_copy(THD *thd)
   { return get_item_copy<Item_func_group_concat>(thd, this); }
+  qsort_cmp2 get_comparator_function_for_distinct();
+  qsort_cmp2 get_comparator_function_for_order_by();
+  uchar* get_record_pointer();
+  uint get_null_bytes();
+
 };
 
 #endif /* ITEM_SUM_INCLUDED */