Bug#35730982: GROUP_CONCAT results were displayed as NULL before 8.0.23

This is a regression from changes made in the fix for bug#32053547.
In this fix, a check for NULL value in group concat evaluation was
moved from Item_func_group_concat::setup() to fix_fields(). The change
made sense, however when a table was subject to const table elimination,
it was no longer optimized early. It would still have been OK, except
that the NULL value check in Item_func_group_concat::add() does not
cover all possible cases and thus a NULL value in an underlying argument
to group_concat might not be detected.

The fix is to reintroduce a NULL value check in the setup() function,
while still keeping the check in fix_fields(). The flag always_null is
replaced by two flags m_null_resolved which is set in fix_fields() and
m_null_executed which is set in setup(). The former handles expressions
that are always NULL, whereas the latter handles expressions that can be
determined to be NULL at start of each execution.

The two flags are also moved to the base class Item_sum so that the same
optimization may easily be added for other aggregate functions.
The flag m_null_executed is reset in Item_sum::cleanup() so that it can
be evaluated properly in start of next execution, when used with
prepared statements.

A specific test case using a prepared statement is added to verify that
the flag is properly cleared for subsequent executions.

Change-Id: I42967ee5908d5e288854463b6b761a049dc7a7ee

Author: roylyseng

mysql-test/r/func_gconcat.result

@@ -1374,3 +1374,43 @@ SET SESSION group_concat_max_len=default;
 # Bug#34703610: Create view with subquery assertion error
 CREATE VIEW v(c0) AS SELECT group_concat((SELECT 1), 1);
 DROP VIEW v;
+# Bug#35730982: GROUP_CONCAT results were displayed as NULL before 8.0.23
+CREATE TABLE customer (
+c_id int NOT NULL,
+c_name varchar(255) NOT NULL,
+PRIMARY KEY (c_id)
+);
+INSERT INTO customer VALUES (1,'Messi'),(2,'Pele'),(3,'Maradona');
+CREATE TABLE employee (
+seq_id bigint NOT NULL,
+c_id int NOT NULL,
+emp_id int NOT NULL,
+emp_name varchar(30) DEFAULT NULL,
+PRIMARY KEY (seq_id)
+);
+INSERT INTO employee VALUES (1,1,10,'Zico'),(2,3,30,'Ronaldo');
+SELECT mc.c_id,
+GROUP_CONCAT(e.emp_name order by e.emp_id separator ',') AS gc
+FROM customer AS mc LEFT JOIN employee AS e
+ON mc.c_id = e.c_id AND e.seq_id = 2
+WHERE mc.c_id = 2
+GROUP BY mc.c_id;
+c_id	gc
+2	NULL
+PREPARE ps FROM
+"SELECT mc.c_id,
+       GROUP_CONCAT(e.emp_name order by e.emp_id separator ',') AS gc
+FROM customer AS mc LEFT JOIN employee AS e
+     ON mc.c_id = e.c_id AND e.seq_id = ?
+WHERE mc.c_id = ?
+GROUP BY mc.c_id";
+SET @seq_id = 2;
+SET @c_id = 2;
+EXECUTE ps USING @seq_id, @c_id;
+c_id	gc
+2	NULL
+SET @c_id = 3;
+EXECUTE ps USING @seq_id, @c_id;
+c_id	gc
+3	Ronaldo
+DROP TABLE customer, employee;

mysql-test/t/func_gconcat.test

@@ -1023,3 +1023,49 @@ SET SESSION group_concat_max_len=default;
 CREATE VIEW v(c0) AS SELECT group_concat((SELECT 1), 1);
 
 DROP VIEW v;
+
+--echo # Bug#35730982: GROUP_CONCAT results were displayed as NULL before 8.0.23
+
+CREATE TABLE customer (
+  c_id int NOT NULL,
+  c_name varchar(255) NOT NULL,
+  PRIMARY KEY (c_id)
+);
+
+INSERT INTO customer VALUES (1,'Messi'),(2,'Pele'),(3,'Maradona');
+
+CREATE TABLE employee (
+  seq_id bigint NOT NULL,
+  c_id int NOT NULL,
+  emp_id int NOT NULL,
+  emp_name varchar(30) DEFAULT NULL,
+  PRIMARY KEY (seq_id)
+);
+
+INSERT INTO employee VALUES (1,1,10,'Zico'),(2,3,30,'Ronaldo');
+
+SELECT mc.c_id,
+       GROUP_CONCAT(e.emp_name order by e.emp_id separator ',') AS gc
+FROM customer AS mc LEFT JOIN employee AS e
+     ON mc.c_id = e.c_id AND e.seq_id = 2
+WHERE mc.c_id = 2
+GROUP BY mc.c_id;
+
+PREPARE ps FROM
+"SELECT mc.c_id,
+       GROUP_CONCAT(e.emp_name order by e.emp_id separator ',') AS gc
+FROM customer AS mc LEFT JOIN employee AS e
+     ON mc.c_id = e.c_id AND e.seq_id = ?
+WHERE mc.c_id = ?
+GROUP BY mc.c_id";
+
+SET @seq_id = 2;
+SET @c_id = 2;
+
+EXECUTE ps USING @seq_id, @c_id;
+
+SET @c_id = 3;
+
+EXECUTE ps USING @seq_id, @c_id;
+
+DROP TABLE customer, employee;

sql/item_sum.cc

@@ -440,7 +440,9 @@ Item_sum::Item_sum(THD *thd, const Item_sum *item)
       base_query_block(item->base_query_block),
       aggr_query_block(item->aggr_query_block),
       allow_group_via_temp_table(item->allow_group_via_temp_table),
-      forced_const(item->forced_const) {
+      forced_const(item->forced_const),
+      m_null_resolved(item->m_null_resolved),
+      m_null_executed(item->m_null_executed) {
   assert(arg_count == item->arg_count);
   with_distinct = item->with_distinct;
   if (item->aggr) {
@@ -867,6 +869,8 @@ void Item_sum::cleanup() {
   Item_result_field::cleanup();
   // forced_const may have been set during optimization, reset it:
   forced_const = false;
+
+  m_null_executed = false;
 }
 
 bool Item_sum::fix_fields(THD *thd, Item **ref [[maybe_unused]]) {
@@ -4227,7 +4231,6 @@ Item_func_group_concat::Item_func_group_concat(THD *thd,
                                                Item_func_group_concat *item)
     : Item_sum(thd, item),
       distinct(item->distinct),
-      always_null(item->always_null),
       m_order_arg_count(item->m_order_arg_count),
       m_field_arg_count(item->m_field_arg_count),
       context(item->context),
@@ -4355,17 +4358,19 @@ void Item_func_group_concat::clear() {
 }
 
 bool Item_func_group_concat::add() {
-  if (always_null) return false;
+  if (m_null_executed) return false;
   THD *thd = current_thd;
   if (copy_funcs(tmp_table_param, thd)) return true;
 
   for (uint i = 0; i < m_field_arg_count; i++) {
-    Item *show_item = args[i];
-    if (show_item->const_item()) continue;
-
-    Field *field = show_item->get_tmp_table_field();
-    if (field && field->is_null_in_record((const uchar *)table->record[0]))
+    Item *item = args[i];
+    if (item->const_for_execution()) {
+      continue;
+    }
+    Field *field = item->get_tmp_table_field();
+    if (field && field->is_null_in_record((const uchar *)table->record[0])) {
       return false;  // Skip row if it contains null
+    }
   }
 
   null_value = false;
@@ -4473,7 +4478,7 @@ bool Item_func_group_concat::fix_fields(THD *thd, Item **ref) {
         item->is_null()) {
       // "is_null()" may cause error:
       if (thd->is_error()) return true;
-      always_null = true;
+      m_null_resolved = true;
     }
   }
 
@@ -4483,7 +4488,7 @@ bool Item_func_group_concat::fix_fields(THD *thd, Item **ref) {
     The "fields" list is not used after the call to setup_order(), however it
     must be recreated during optimization to create tmp table columns.
   */
-  if (m_order_arg_count > 0 && !always_null &&
+  if (m_order_arg_count > 0 && !m_null_resolved &&
       setup_order(thd, Ref_item_array(args, arg_count), context->table_list,
                   &fields, order_array.begin()))
     return true;
@@ -4503,8 +4508,10 @@ bool Item_func_group_concat::setup(THD *thd) {
   */
   if (table != nullptr || tree != nullptr) return false;
 
-  // Nothing to set up if always NULL:
-  if (always_null) return false;
+  // If resolved as NULL, execution is always NULL
+  m_null_executed = m_null_resolved;
+  // Nothing to set up if value is NULL:
+  if (m_null_executed) return false;
 
   assert(thd->lex->current_query_block() == aggr_query_block);
 
@@ -4535,7 +4542,13 @@ bool Item_func_group_concat::setup(THD *thd) {
 
   // First add the fields from the concat field list
   for (uint i = 0; i < m_field_arg_count; i++) {
-    fields.push_back(args[i]);
+    Item *item = args[i];
+    fields.push_back(item);
+    if (item->const_for_execution()) {
+      if (item->is_null()) m_null_executed = true;
+      if (thd->is_error()) return true;
+      if (m_null_executed) return false;
+    }
   }
   // Then prepend the ordered fields not already in the "fields" list
   for (uint i = 0; i < m_order_arg_count; i++) {

sql/item_sum.h

@@ -505,6 +505,12 @@ class Item_sum : public Item_func {
     The value must be reset to false after execution.
   */
   bool forced_const{false};
+
+  /// true if the function is resolved to be always NULL
+  bool m_null_resolved{false};
+  /// true if the function is determined to be NULL at start of execution
+  bool m_null_executed{false};
+
   static ulonglong ram_limitation(THD *thd);
 
  public:
@@ -2095,8 +2101,6 @@ class Item_func_group_concat final : public Item_sum {
 
   /// True if GROUP CONCAT has the DISTINCT attribute
   bool distinct;
-  /// True if the result is always NULL
-  bool always_null{false};
   /// The number of ORDER BY items.
   uint m_order_arg_count;
   /// The number of selected items, aka the concat field list