Fixing ORDER BY for JSON_ARRAYAGG10.5-mdev11563

2 files changed, 84 insertions, 5 deletions

diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index cabc6493f54..8e49ae57d84 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -3656,6 +3656,70 @@ int group_concat_key_cmp_with_order(void* arg, const void* key1,
 }
 
 
+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())
+      continue;
+    /*
+      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
+
+      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;
+
+    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 ((*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;
+}
+
+
 /**
   Append data from current leaf to item->result.
 */
@@ -4053,7 +4117,10 @@ 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,
+            (exclude_nulls ?
+             group_concat_key_cmp_with_order :
+             group_concat_key_cmp_with_order_with_nulls), NULL,
             (void*) this, MYF(MY_THREAD_SPECIFIC));
   DBUG_ASSERT(tree->size_of_element == st.tree.size_of_element);
   st.table= table;
@@ -4133,7 +4200,9 @@ bool Item_func_group_concat::add()
 
   if (!distinct && row_eligible && tree &&
       insert_to_order_tree(row_str_len,
-                           table->record[0] + table->s->null_bytes))
+                           exclude_nulls ?
+                           table->record[0] + table->s->null_bytes :
+                           table->record[0]))
     return 1;
   /*
     In case of GROUP_CONCAT with DISTINCT or ORDER BY (or both) don't dump the
@@ -4223,7 +4292,7 @@ String *Item_func_group_concat::get_value_for_arg(String *res, Item *arg,
         *is_null= TRUE;
       res= field->val_str(res,
                           key + offset + (exclude_nulls ? 0 :
-                                          table->s->null_bytes +offset));
+                                          table->s->null_bytes));
     }
     else
       res= arg->val_str(res);
@@ -4422,8 +4491,10 @@ 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 + (exclude_nulls ? 0: table->s->null_bytes),
+              (exclude_nulls ?
+              group_concat_key_cmp_with_order:
+              group_concat_key_cmp_with_order_with_nulls), NULL, (void*) this,
               MYF(MY_THREAD_SPECIFIC));
     tree_len= 0;
   }
diff --git a/sql/item_sum.h b/sql/item_sum.h
index c6aac0e466d..e60509a4cc9 100644
--- a/sql/item_sum.h
+++ b/sql/item_sum.h
@@ -1845,6 +1845,11 @@ 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);
@@ -1914,6 +1919,9 @@ protected:
                                                            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);