MDEV-22640, MDEV-22449, MDEV-21528 JSON_ARRAYAGG crashes with NULL values.

We have to include NULL in the result which the GOUP_CONCAT doesn't
always do. Also converting should be done into another String instance
as these can be same.

4 files changed, 32 insertions, 28 deletions

diff --git a/sql/item_jsonfunc.cc b/sql/item_jsonfunc.cc
index 134c2573ea7..45f57d99011 100644
--- a/sql/item_jsonfunc.cc
+++ b/sql/item_jsonfunc.cc
@@ -3621,12 +3621,12 @@ int Arg_comparator::compare_e_json_str_basic(Item *j, Item *s)
 }
 
 
-String* Item_func_json_arrayagg::convert_to_json(Item *item, String *res)
+String* Item_func_json_arrayagg::convert_to_json(Item *item)
 {
   String tmp;
-  res->length(0);
-  append_json_value(res, item, &tmp);
-  return res;
+  m_tmp_json.length(0);
+  append_json_value(&m_tmp_json, item, &tmp);
+  return &m_tmp_json;
 }
 
 
diff --git a/sql/item_jsonfunc.h b/sql/item_jsonfunc.h
index f1afaf6cc31..e5676768854 100644
--- a/sql/item_jsonfunc.h
+++ b/sql/item_jsonfunc.h
@@ -537,7 +537,15 @@ public:
 
 class Item_func_json_arrayagg : public Item_func_group_concat
 {
+protected:
+  /*
+    Overrides Item_func_group_concat::skip_nulls()
+    NULL-s should be added to the result as JSON null value.
+  */
+  virtual bool skip_nulls() const { return false; }
+
 public:
+  String m_tmp_json; /* Used in convert_to_json. */
   Item_func_json_arrayagg(THD *thd, Name_resolution_context *context_arg,
                           bool is_distinct, List<Item> *is_select,
                           const SQL_I_List<ORDER> &is_order, String *is_separator,
@@ -552,14 +560,9 @@ public:
   const char *func_name() const { return "json_arrayagg("; }
   enum Sumfunctype sum_func() const {return JSON_ARRAYAGG_FUNC;}
 
-  String* convert_to_json(Item *item, String *str);
+  String* convert_to_json(Item *item);
   String* val_str(String *str);
 
-  /* Overrides Item_func_group_concat::add() */
-  bool add()
-  {
-    return Item_func_group_concat::add(false);
-  }
   Item *get_copy(THD *thd)
   { return get_item_copy<Item_func_json_arrayagg>(thd, this); }
 };
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index b676186f571..0ce5352ccfb 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -3674,20 +3674,18 @@ int dump_leaf_key(void* key_arg, element_count count __attribute__((unused)),
       else
         res= (*arg)->val_str(&tmp);
     }
-    if (res)
+    if (item->sum_func() == Item_sum::JSON_ARRAYAGG_FUNC)
     {
-      if (item->sum_func() == Item_sum::JSON_ARRAYAGG_FUNC)
-      {
-        /*
-          JSON_ARRAYAGG needs to convert the type into valid JSON before
-          appending it to the result
-        */
-        Item_func_json_arrayagg *arrayagg= (Item_func_json_arrayagg *) item_arg;
-        res= arrayagg->convert_to_json(*arg, res);
-      }
+      /*
+        JSON_ARRAYAGG needs to convert the type into valid JSON before
+        appending it to the result
+      */
+      Item_func_json_arrayagg *arrayagg= (Item_func_json_arrayagg *) item_arg;
+      res= arrayagg->convert_to_json(*arg);
+    }
 
+    if (res)
       result->append(*res);
-    }
   }
 
   if (item->limit_clause)
@@ -4151,13 +4149,10 @@ bool Item_func_group_concat::setup(THD *thd)
     Item *item= args[i];
     if (list.push_back(item, thd->mem_root))
       DBUG_RETURN(TRUE);
-    if (item->const_item())
+    if (item->const_item() && item->is_null() && skip_nulls())
     {
-      if (item->is_null())
-      {
-        always_null= 1;
-        DBUG_RETURN(FALSE);
-      }
+      always_null= 1;
+      DBUG_RETURN(FALSE);
     }
   }
 
diff --git a/sql/item_sum.h b/sql/item_sum.h
index 7ff71c84209..7c69dea4a9a 100644
--- a/sql/item_sum.h
+++ b/sql/item_sum.h
@@ -1918,6 +1918,12 @@ protected:
 
   bool repack_tree(THD *thd);
 
+  /*
+    Says whether the function should skip NULL arguments
+    or add them to the result.
+    Redefined in JSON_ARRAYAGG.
+  */
+  virtual bool skip_nulls() const { return true; }
 public:
   // Methods used by ColumnStore
   bool get_distinct() const { return distinct; }
@@ -1947,7 +1953,7 @@ public:
   void clear();
   bool add()
   {
-    return add(true);
+    return add(skip_nulls());
   }
   void reset_field() { DBUG_ASSERT(0); }        // not used
   void update_field() { DBUG_ASSERT(0); }       // not used