JSON_OBJECTAGG and JSON_ARRAYAGG corrupt their result when the aggregate is evaluated more than once

Both JSON aggregates close their result in the call that hands it over rather
than when the group ends. Nothing stops that call happening twice, and the
buffer being closed belongs to the aggregate and survives between calls, so a
statement that evaluates the aggregate more than once for the same group gets
a value that has been closed once per evaluation.

JSON_OBJECTAGG returns a document with an extra closing brace, which is
not JSON at all. JSON_ARRAYAGG returns a document wrapped in an extra pair
of brackets, which is valid JSON of the wrong value - the quieter of the two
failures, since nothing downstream has any reason to reject it.

Neither raises a warning or an error.

How to repeat

Any statement that reads the aggregate twice will do; HAVING on the alias
is the shortest.

CREATE TABLE t1 (a VARCHAR(10));

INSERT INTO t1 VALUES ('x'),('y');

SELECT JSON_OBJECTAGG(a,a) AS v FROM t1;

SELECT JSON_OBJECTAGG(a,a) AS v FROM t1 HAVING v LIKE '%';

v

{"x":"x", "y":"y"}          <- correct

v

{"x":"x", "y":"y"}}         <- one evaluation later

The count follows the number of evaluations:

SELECT JSON_OBJECTAGG(a,a) AS v FROM t1 HAVING v LIKE '%' AND v LIKE '{%';

v

{"x":"x", "y":"y"}}}

JSON_ARRAYAGG under the same statement:

SELECT JSON_ARRAYAGG(a) AS v FROM t1;

SELECT JSON_ARRAYAGG(a) AS v FROM t1 HAVING v LIKE '%';

v

["x","y"]                   <- correct

v

[["x","y"]]                 <- one evaluation later

Every group of a GROUP BY is affected, not just the first:

CREATE TABLE t2 (g INT, a VARCHAR(10));

INSERT INTO t2 VALUES (1,'x'),(1,'y'),(2,'z');

SELECT g, JSON_OBJECTAGG(a,a) AS v FROM t2 GROUP BY g HAVING v LIKE '%';

g	v

1	{"x":"x", "y":"y"}}

2	{"z":"z"}}

Root cause

An aggregate is built up across three calls: clear() at the start of a
group, add() once per row, and val_str() when the result is wanted.
The text accumulates in a buffer that belongs to the item, because there is
nowhere else to keep it between rows. Closing that buffer in val_str()
therefore modifies it, and the modification is not undone.

/* sql/item_jsonfunc.cc */

String* Item_func_json_objectagg::val_str(String* str)

{

  DBUG_ASSERT(fixed());

  if (null_value)

    return 0;

  result.append('}');    /* 'result' is a member; nothing guards this */

  return &result;

}

JSON_ARRAYAGG reaches the same state by a different route. It asks its
parent for the concatenated elements, which hands back the parent's own
buffer, and then puts the brackets around what is in that buffer:

/* sql/item_jsonfunc.cc */

String* Item_func_json_arrayagg::val_str(String *str)

{

  if ((str= Item_func_group_concat::val_str(str)))   /* returns &result */

  {

    String s;

    ...

    s.append('[');

    s.swap(*str);

    str->append(s);

    str->append(']');

  }

  return str;

}

So the second call finds ["x","y"] where it expects "x","y" and wraps
that.

The parent class has the same exposure and guards against it. GROUP_CONCAT
keeps a flag saying the result has already been produced, and a second call
returns the buffer without rebuilding it:

/* sql/item_sum.cc */

String* Item_func_group_concat::val_str(String* str)

{

  ...

  if (!result_finalized) // Result yet to be written.

  { ... }

  return &result;

}

That flag protects the elements, which is why the elements are not duplicated
in the output above. It does not protect the brackets or the brace, because
those are written by the derived classes after the parent has returned.

Affected versions

Both aggregates were added by MDEV-16620 (commit ba8e5e689c8,
2019-10-14), first released in 10.5.0, and both defects were present in that
commit:

/* sql/item_jsonfunc.cc @ ba8e5e689c8 */

String* Item_func_json_objectagg::val_str(String* str)

{

  DBUG_ASSERT(fixed == 1);

  if (null_value)

    return 0;

  result.append("}");

  return &result;

}

String* Item_func_json_arrayagg::val_str(String *str)

{

  str= Item_func_group_concat::val_str(str);

  String s;

  s.append('[');

  s.swap(*str);

  str->append(s);

  str->append(']');

  return str;

}

Every release from 10.5.0 onward is therefore expected to be affected.

Verified on 10.11 only (10.11.19-MariaDB). Other branches were not tested.

Note

Found while testing an unrelated change to the JSON aggregates. No existing
test evaluates either aggregate more than once in a statement, which is why
neither defect has shown up.