MDEV-38716: Server crashes in TABLE::evaluate_update_default_function

This patch fixes two issues:

Problem (1)
A regression from MDEV-36290 would cause the server to crash when one
uses TIMESTAMP or DATETIME types with option ON UPDATE. The root cause
of this is actually from problem (2) described below, MDEV-36290 simply
forced the bug to be revealed.

Problem (2)
After a table is rebuilt (i.e. via ALTER TABLE .. FORCE), any fields
which use ON UPDATE lose the update trigger after the ALTER. This is
because recreated fields (i.e those in alter_info->create_list) had no
checks to ensure their representation in the target table's
default_fields list. The intent is provided by the following code
comment:

/*
  Update the set of auto-update fields to contain only the new fields
  added to the table. Only these fields should be updated automatically.
  Old fields keep their current values, and therefore should not be
  present in the set of autoupdate fields.
*/

However, fields with trigger-based default values were left
unconsidered.

Linking back to problem (1), the crash happened because of the
following MDEV-36290 changes:

diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 13ed810cac2..5cc74785ef7 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -12737,7 +12737,12 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
     }
   }
   if (dfield_ptr)
-    *dfield_ptr= NULL;
+  {
+    if (dfield_ptr == to->default_field)
+      to->default_field= 0; // No default fields left
+    else
+      *dfield_ptr= NULL; // Mark end of default field pointers
+  }

That is, the destination table's default_field becomes nullified when
no default_field changes exist. However, because ON UPDATE
default_fields were not moved to the destination table, the server
would crash with a segfault when updating the records in the
post-ALTER table (in TABLE::evaluate_update_default_function()) because
it would de-reference the nullified pointer in default_field.

The fix for these problems is the same. ON UPDATE based default_fields
are added back to the destination table after the actual data copy.
Note this must be done after the data copy for fields which have both
DEFAULT and ON UPDATE set for timestamps.

Additionally, this patch removes the set/reset of
table->s->default_fields from TABLE::evaluate_update_default_function()
because it is no longer used when updating default fields. The
functions that use default_field use the NULL terminator at the end
of the list to stop iteration.

This patch adds a test case to main.temp_table to test the case of
a table with a field column with a NULL default value but
ON UPDATE CURRENT_TIMESTAMP. Existing MTR tests have cases for
DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP (i.e. any test
which deals with upgrades, e.g. main.upgrade_mdev_24363 and
main.mysql_upgrade_mysql_json_with_plugin_loaded).

Reviewed-by: Monty <monty@mariadb.com>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>

mysql-test/main/temp_table.result

@@ -721,3 +721,30 @@ pk
 0
 1
 DROP TABLE t;
+#
+# MDEV-38716: Server crashes in TABLE::evaluate_update_default_function
+#
+# Test case where ALTER doesn't add any new columns with default values
+CREATE TEMPORARY TABLE tmp_38716 (a timestamp null on update current_timestamp(6), b int);
+ALTER TABLE tmp_38716 FORCE;
+INSERT INTO tmp_38716 VALUES (NULL,1);
+UPDATE tmp_38716 SET b = 2;
+SELECT * from tmp_38716;
+a	b
+2001-01-01 10:20:30	2
+DROP TABLE tmp_38716;
+# Test case where ALTER adds a new column with a default value
+CREATE TEMPORARY TABLE tmp_38716_2 (a timestamp null on update current_timestamp(6), b int default 0);
+INSERT INTO tmp_38716_2 (b) VALUES (10);
+ALTER TABLE tmp_38716_2 ADD COLUMN c INT DEFAULT (b+1), FORCE;
+# Update should trigger ON UPDATE and set a to current timestamp where b=11
+UPDATE tmp_38716_2 SET b = 11;
+SELECT * from tmp_38716_2 where b=11;
+a	b	c
+2001-01-01 10:20:30	11	11
+# New row should have a=NULL and c=21
+INSERT INTO tmp_38716_2 (b) VALUES (20);
+SELECT * FROM tmp_38716_2 where b=20;
+a	b	c
+NULL	20	21
+DROP TABLE tmp_38716_2;

mysql-test/main/temp_table.test

@@ -785,3 +785,30 @@ INSERT INTO t VALUES (1);
 COMMIT;
 SELECT * FROM t ORDER BY pk;
 DROP TABLE t;
+
+--echo #
+--echo # MDEV-38716: Server crashes in TABLE::evaluate_update_default_function
+--echo #
+
+--echo # Test case where ALTER doesn't add any new columns with default values
+CREATE TEMPORARY TABLE tmp_38716 (a timestamp null on update current_timestamp(6), b int);
+ALTER TABLE tmp_38716 FORCE;
+INSERT INTO tmp_38716 VALUES (NULL,1);
+UPDATE tmp_38716 SET b = 2;
+SELECT * from tmp_38716;
+DROP TABLE tmp_38716;
+
+--echo # Test case where ALTER adds a new column with a default value
+CREATE TEMPORARY TABLE tmp_38716_2 (a timestamp null on update current_timestamp(6), b int default 0);
+INSERT INTO tmp_38716_2 (b) VALUES (10);
+ALTER TABLE tmp_38716_2 ADD COLUMN c INT DEFAULT (b+1), FORCE;
+
+--echo # Update should trigger ON UPDATE and set a to current timestamp where b=11
+UPDATE tmp_38716_2 SET b = 11;
+SELECT * from tmp_38716_2 where b=11;
+
+--echo # New row should have a=NULL and c=21
+INSERT INTO tmp_38716_2 (b) VALUES (20);
+SELECT * FROM tmp_38716_2 where b=20;
+
+DROP TABLE tmp_38716_2;

sql/sql_table.cc

@@ -12623,7 +12623,6 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
   sql_mode_t save_sql_mode= thd->variables.sql_mode;
   ulonglong prev_insert_id, time_to_report_progress;
   Field **dfield_ptr= to->default_field;
-  uint save_to_s_default_fields= to->s->default_fields;
   bool make_versioned= !from->versioned() && to->versioned();
   bool make_unversioned= from->versioned() && !to->versioned();
   bool keep_versioned= from->versioned() && to->versioned();
@@ -12684,7 +12683,6 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
   List_iterator<Create_field> it(alter_info->create_list);
   Create_field *def;
   copy_end=copy;
-  to->s->default_fields= 0;
   error= 1;
   if (to->s->table_type == TABLE_TYPE_SEQUENCE &&
       from->file->ha_table_flags() & HA_STATS_RECORDS_IS_EXACT &&
@@ -12730,16 +12728,19 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
         present in the set of autoupdate fields.
       */
       if ((*ptr)->default_value)
-      {
         *(dfield_ptr++)= *ptr;
-        ++to->s->default_fields;
-      }
     }
   }
+
+  /*
+    Mark the end of the default field list. Note that this is a temporary
+    state; fields with ON UPDATE defaults will be added back to this list
+    after the data copy is complete.
+  */
   if (dfield_ptr)
   {
     if (dfield_ptr == to->default_field)
-      to->default_field= 0; // No default fields left
+      to->default_field= NULL; // No default fields left
     else
       *dfield_ptr= NULL; // Mark end of default field pointers
   }
@@ -13021,6 +13022,56 @@ copy_data_between_tables(THD *thd, TABLE *from, TABLE *to,
   cleanup_done= 1;
   to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 
+  /*
+    To prevent this data copy from re-writing previously set default values,
+    this function previously has overridden to->default_fields to only account
+    for new fields added to the table (i.e those which should get new default
+    values). Now that the data copy is done, the to->default_field list needs
+    to be restored to include all fields that have default values to fill.
+
+    First, if to->default_field is NULL, it means it was nullified to prevent
+    trying to update any default values during the copy process because there
+    were no new fields added with default values to fill in. The list needs to
+    be restored to the original location so future records in this session can
+    get default values.
+
+    Then, because to->default_field was reset and now only considers new
+    fields, it is missing fields that have ON UPDATE clauses (i.e. TIMESTAMP
+    and DATETIME). So add those back in.
+
+    Notes:
+     * The code is similar to the dfield_ptr modification earlier in the
+       function
+     * This must be done before online alter execution, as it unpacks row
+       data, which requires the table's default_fields to be set up
+  */
+  if (!to->default_field)
+    to->default_field= dfield_ptr;
+#ifndef DBUG_OFF
+  else
+  {
+    /*
+      If to->default_field exists, dfield_ptr should point to the NULL list
+      terminator in the default_field list.
+    */
+    DBUG_ASSERT(dfield_ptr && !*dfield_ptr);
+  }
+#endif
+  if (dfield_ptr)
+  {
+    it.rewind();
+    for (Field **ptr= to->field; *ptr; ptr++)
+    {
+      def= it++;
+      if (def->field && def->field->has_update_default_function())
+        *(dfield_ptr++)= *ptr;
+    }
+    if (dfield_ptr == to->default_field)
+      to->default_field= NULL; // No default fields
+    else
+      *dfield_ptr= NULL; // Mark end of default field pointers
+  }
+
 #ifdef HAVE_REPLICATION
   if (online && error < 0)
   {
@@ -13145,7 +13196,6 @@ end:
   *copied= found_count;
   *deleted=delete_count;
   to->file->ha_release_auto_increment();
-  to->s->default_fields= save_to_s_default_fields;
 
   if (!cleanup_done)
   {