MDEV-23295 ROW_FORMAT mismatch in instant ALTER TABLE

An instant ADD/DROP/reorder column could create a dummy table
object with the wrong ROW_FORMAT when innodb_default_row_format
was changed between CREATE TABLE and ALTER TABLE.

prepare_inplace_alter_table_dict(): If we had promised that
ALGORITHM=INPLACE is supported, we must preserve the ROW_FORMAT.

dict_table_t::prepare_instant(): Add debug assertions to catch
ROW_FORMAT mismatch.

The rest of the changes are related to adding
Alter_inplace_info::inplace_supported to cache the return value of
handler::check_if_supported_inplace_alter().

mysql-test/suite/innodb/r/default_row_format_alter,compact.rdiff

+--- default_row_format_alter.result
++++ default_row_format_alter,compact.reject
+@@ -91,6 +91,6 @@
+ ALTER TABLE t1 ADD b INT;
+ SELECT ROW_FORMAT FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME='t1';
+ ROW_FORMAT
+-Dynamic
++Compact
+ DROP TABLE t1;
+ SET GLOBAL innodb_default_row_format = @row_format;

mysql-test/suite/innodb/r/default_row_format_alter,redundant.rdiff

+--- default_row_format_alter.result
++++ default_row_format_alter,compact.reject
+@@ -91,6 +91,6 @@
+ ALTER TABLE t1 ADD b INT;
+ SELECT ROW_FORMAT FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME='t1';
+ ROW_FORMAT
+-Dynamic
++Redundant
+ DROP TABLE t1;
+ SET GLOBAL innodb_default_row_format = @row_format;

mysql-test/suite/innodb/r/default_row_format_alter.result

@@ -82,4 +82,15 @@ SHOW TABLE STATUS LIKE 't1';
 Name	Engine	Version	Row_format	Rows	Avg_row_length	Data_length	Max_data_length	Index_length	Data_free	Auto_increment	Create_time	Update_time	Check_time	Collation	Checksum	Create_options	Comment	Max_index_length	Temporary
 t1	InnoDB	#	Compact	#	#	#	#	#	#	NULL	#	#	NULL	latin1_swedish_ci	NULL			0	N
 DROP TABLE t1;
+#
+# MDEV-23295 Assertion fields[i].same(instant.fields[i]) failed
+#
+SET GLOBAL innodb_default_row_format = @row_format;
+CREATE TABLE t1 (a char(8)) ENGINE=InnoDB DEFAULT CHARSET utf8;
+SET GLOBAL innodb_default_row_format= COMPACT;
+ALTER TABLE t1 ADD b INT;
+SELECT ROW_FORMAT FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME='t1';
+ROW_FORMAT
+Dynamic
+DROP TABLE t1;
 SET GLOBAL innodb_default_row_format = @row_format;

mysql-test/suite/innodb/t/default_row_format_alter.test

 --source include/have_innodb.inc
+--source include/innodb_row_format.inc
 
 SET @row_format = @@GLOBAL.innodb_default_row_format;
 
@@ -95,4 +96,14 @@ ALTER TABLE t1 DROP INDEX k1;
 SHOW TABLE STATUS LIKE 't1';
 DROP TABLE t1;
 
+--echo #
+--echo # MDEV-23295 Assertion fields[i].same(instant.fields[i]) failed
+--echo #
+SET GLOBAL innodb_default_row_format = @row_format;
+CREATE TABLE t1 (a char(8)) ENGINE=InnoDB DEFAULT CHARSET utf8;
+SET GLOBAL innodb_default_row_format= COMPACT;
+ALTER TABLE t1 ADD b INT;
+SELECT ROW_FORMAT FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME='t1';
+DROP TABLE t1;
+
 SET GLOBAL innodb_default_row_format = @row_format;

sql/handler.h

@@ -2440,6 +2440,9 @@ class Alter_inplace_info
   /** true for online operation (LOCK=NONE) */
   bool online;
 
+  /** which ALGORITHM and LOCK are supported by the storage engine */
+  enum_alter_inplace_result inplace_supported;
+
   /**
      Can be set by handler to describe why a given operation cannot be done
      in-place (HA_ALTER_INPLACE_NOT_SUPPORTED) or why it cannot be done

sql/sql_alter.cc

 /* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
-   Copyright (c) 2016, 2018, MariaDB Corporation
+   Copyright (c) 2016, 2020, MariaDB Corporation
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -127,10 +127,10 @@ const char* Alter_info::lock() const
 }
 
 
-bool Alter_info::supports_algorithm(THD *thd, enum_alter_inplace_result result,
+bool Alter_info::supports_algorithm(THD *thd,
                                     const Alter_inplace_info *ha_alter_info)
 {
-  switch (result) {
+  switch (ha_alter_info->inplace_supported) {
   case HA_ALTER_INPLACE_EXCLUSIVE_LOCK:
   case HA_ALTER_INPLACE_SHARED_LOCK:
   case HA_ALTER_INPLACE_NO_LOCK:
@@ -171,10 +171,10 @@ bool Alter_info::supports_algorithm(THD *thd, enum_alter_inplace_result result,
 }
 
 
-bool Alter_info::supports_lock(THD *thd, enum_alter_inplace_result result,
+bool Alter_info::supports_lock(THD *thd,
                                const Alter_inplace_info *ha_alter_info)
 {
-  switch (result) {
+  switch (ha_alter_info->inplace_supported) {
   case HA_ALTER_INPLACE_EXCLUSIVE_LOCK:
     // If SHARED lock and no particular algorithm was requested, use COPY.
     if (requested_lock == Alter_info::ALTER_TABLE_LOCK_SHARED &&

sql/sql_alter.h

 /* Copyright (c) 2010, 2014, Oracle and/or its affiliates.
-   Copyright (c) 2013, 2018, MariaDB Corporation.
+   Copyright (c) 2013, 2020, MariaDB Corporation.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -204,29 +204,26 @@ class Alter_info
      with the specified user alter algorithm.
 
      @param  thd            Thread handle
-     @param  result         Operation supported for inplace alter
      @param  ha_alter_info  Structure describing changes to be done
                             by ALTER TABLE and holding data during
                             in-place alter
      @retval false  Supported operation
      @retval true   Not supported value
   */
-  bool supports_algorithm(THD *thd, enum_alter_inplace_result result,
+  bool supports_algorithm(THD *thd,
                           const Alter_inplace_info *ha_alter_info);
 
   /**
      Check whether the given result can be supported
      with the specified user lock type.
 
-     @param  result         Operation supported for inplace alter
      @param  ha_alter_info  Structure describing changes to be done
                             by ALTER TABLE and holding data during
                             in-place alter
      @retval false  Supported lock type
      @retval true   Not supported value
   */
-  bool supports_lock(THD *thd, enum_alter_inplace_result result,
-                     const Alter_inplace_info *ha_alter_info);
+  bool supports_lock(THD *thd, const Alter_inplace_info *ha_alter_info);
 
   /**
     Return user requested algorithm. If user does not specify

sql/sql_table.cc

@@ -7560,7 +7560,6 @@ static bool is_inplace_alter_impossible(TABLE *table,
   @param ha_alter_info      Structure describing ALTER TABLE to be carried
                             out and serving as a storage place for data
                             used during different phases.
-  @param inplace_supported  Enum describing the locking requirements.
   @param target_mdl_request Metadata request/lock on the target table name.
   @param alter_ctx          ALTER TABLE runtime context.
 
@@ -7585,7 +7584,6 @@ static bool mysql_inplace_alter_table(THD *thd,
                                       TABLE *table,
                                       TABLE *altered_table,
                                       Alter_inplace_info *ha_alter_info,
-                                      enum_alter_inplace_result inplace_supported,
                                       MDL_request *target_mdl_request,
                                       Alter_table_ctx *alter_ctx)
 {
@@ -7595,6 +7593,8 @@ static bool mysql_inplace_alter_table(THD *thd,
   Alter_info *alter_info= ha_alter_info->alter_info;
   bool reopen_tables= false;
   bool res;
+  const enum_alter_inplace_result inplace_supported=
+    ha_alter_info->inplace_supported;
 
   DBUG_ENTER("mysql_inplace_alter_table");
 
@@ -10121,19 +10121,19 @@ do_continue:;
     if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_NONE)
       ha_alter_info.online= true;
     // Ask storage engine whether to use copy or in-place
-    enum_alter_inplace_result inplace_supported=
+    ha_alter_info.inplace_supported=
       table->file->check_if_supported_inplace_alter(&altered_table,
                                                     &ha_alter_info);
 
-    if (alter_info->supports_algorithm(thd, inplace_supported, &ha_alter_info) ||
-        alter_info->supports_lock(thd, inplace_supported, &ha_alter_info))
+    if (alter_info->supports_algorithm(thd, &ha_alter_info) ||
+        alter_info->supports_lock(thd, &ha_alter_info))
     {
       cleanup_table_after_inplace_alter(&altered_table);
       goto err_new_table_cleanup;
     }
 
     // If SHARED lock and no particular algorithm was requested, use COPY.
-    if (inplace_supported == HA_ALTER_INPLACE_EXCLUSIVE_LOCK &&
+    if (ha_alter_info.inplace_supported == HA_ALTER_INPLACE_EXCLUSIVE_LOCK &&
         alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_SHARED &&
          alter_info->algorithm(thd) ==
                  Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT &&
@@ -10141,7 +10141,7 @@ do_continue:;
                  Alter_info::ALTER_TABLE_ALGORITHM_DEFAULT)
       use_inplace= false;
 
-    if (inplace_supported == HA_ALTER_INPLACE_NOT_SUPPORTED)
+    if (ha_alter_info.inplace_supported == HA_ALTER_INPLACE_NOT_SUPPORTED)
       use_inplace= false;
 
     if (use_inplace)
@@ -10154,7 +10154,7 @@ do_continue:;
       */
       thd->count_cuted_fields = CHECK_FIELD_WARN;
       int res= mysql_inplace_alter_table(thd, table_list, table, &altered_table,
-                                         &ha_alter_info, inplace_supported,
+                                         &ha_alter_info,
                                          &target_mdl_request, &alter_ctx);
       thd->count_cuted_fields= save_count_cuted_fields;
       my_free(const_cast<uchar*>(frm.str));

storage/innobase/handler/handler0alter.cc

@@ -238,6 +238,9 @@ inline void dict_table_t::prepare_instant(const dict_table_t& old,
 	DBUG_ASSERT(old.n_cols == old.n_def);
 	DBUG_ASSERT(n_cols == n_def);
 	DBUG_ASSERT(old.supports_instant());
+	DBUG_ASSERT(not_redundant() == old.not_redundant());
+	DBUG_ASSERT(DICT_TF_HAS_ATOMIC_BLOBS(flags)
+		    == DICT_TF_HAS_ATOMIC_BLOBS(old.flags));
 	DBUG_ASSERT(!persistent_autoinc
 		    || persistent_autoinc == old.persistent_autoinc);
 	/* supports_instant() does not necessarily hold here,
@@ -6206,6 +6209,15 @@ prepare_inplace_alter_table_dict(
 
 	user_table = ctx->new_table;
 
+	if (ha_alter_info->inplace_supported == HA_ALTER_INPLACE_INSTANT) {
+		/* If we promised ALGORITHM=INSTANT capability, we must
+		retain the original ROW_FORMAT of the table. */
+		flags = (user_table->flags & (DICT_TF_MASK_COMPACT
+					      | DICT_TF_MASK_ATOMIC_BLOBS))
+			| (flags & ~(DICT_TF_MASK_COMPACT
+				     | DICT_TF_MASK_ATOMIC_BLOBS));
+	}
+
 	trx_start_if_not_started_xa(ctx->prebuilt->trx, true);
 
 	if (ha_alter_info->handler_flags