MDEV-5667 online alter and changed field/index options

use the Alter_inplace_info::ALTER_COLUMN_OPTION flag if
field/column flags were altered.

change ha_example to use check_if_supported_inplace_alter() instead
of obsolete check_if_incompatible_data()

mysql-test/r/plugin.result

@@ -127,7 +127,7 @@ drop table t1;
 SET @OLD_SQL_MODE=@@SQL_MODE;
 SET SQL_MODE='IGNORE_BAD_TABLE_OPTIONS';
 #illegal value fixed
-CREATE TABLE t1 (a int) ENGINE=example ULL=10000000000000000000 one_or_two='ttt' YESNO=SSS;
+CREATE TABLE t1 (a int, b int) ENGINE=example ULL=10000000000000000000 one_or_two='ttt' YESNO=SSS;
 Warnings:
 Warning	1912	Incorrect value '10000000000000000000' for option 'ULL'
 Warning	1912	Incorrect value 'ttt' for option 'one_or_two'
@@ -135,7 +135,8 @@ Warning	1912	Incorrect value 'SSS' for option 'YESNO'
 show create table t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (
-  `a` int(11) DEFAULT NULL
+  `a` int(11) DEFAULT NULL,
+  `b` int(11) DEFAULT NULL
 ) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ULL`=10000000000000000000 `one_or_two`='ttt' `YESNO`=SSS `VAROPT`='5'
 #alter table
 alter table t1 ULL=10000000;
@@ -144,7 +145,8 @@ Note	1105	EXAMPLE DEBUG: ULL 4294967290 -> 10000000
 show create table t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (
-  `a` int(11) DEFAULT NULL
+  `a` int(11) DEFAULT NULL,
+  `b` int(11) DEFAULT NULL
 ) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `one_or_two`='ttt' `YESNO`=SSS `VAROPT`='5' `ULL`=10000000
 alter table t1 change a a int complex='c,c,c';
 Warnings:
@@ -152,15 +154,15 @@ Note	1105	EXAMPLE DEBUG: Field `a` COMPLEX '(null)' -> 'c,c,c'
 show create table t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (
-  `a` int(11) DEFAULT NULL `complex`='c,c,c'
+  `a` int(11) DEFAULT NULL `complex`='c,c,c',
+  `b` int(11) DEFAULT NULL
 ) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `one_or_two`='ttt' `YESNO`=SSS `VAROPT`='5' `ULL`=10000000
 alter table t1 one_or_two=two;
-Warnings:
-Note	1105	EXAMPLE DEBUG: Field `a` COMPLEX 'c,c,c' -> 'c,c,c'
 show create table t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (
-  `a` int(11) DEFAULT NULL `complex`='c,c,c'
+  `a` int(11) DEFAULT NULL `complex`='c,c,c',
+  `b` int(11) DEFAULT NULL
 ) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `YESNO`=SSS `VAROPT`='5' `ULL`=10000000 `one_or_two`=two
 drop table t1;
 #illegal value error
@@ -204,8 +206,6 @@ t1	CREATE TABLE `t1` (
   `a` int(11) DEFAULT NULL
 ) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `varopt`=15
 alter table t1 varopt=default;
-Warnings:
-Note	1105	EXAMPLE DEBUG: Field `a` COMPLEX '(null)' -> '(null)'
 show create table t1;
 Table	Create Table
 t1	CREATE TABLE `t1` (

mysql-test/t/plugin.test

@@ -121,7 +121,7 @@ SET @OLD_SQL_MODE=@@SQL_MODE;
 SET SQL_MODE='IGNORE_BAD_TABLE_OPTIONS';
 
 --echo #illegal value fixed
-CREATE TABLE t1 (a int) ENGINE=example ULL=10000000000000000000 one_or_two='ttt' YESNO=SSS;
+CREATE TABLE t1 (a int, b int) ENGINE=example ULL=10000000000000000000 one_or_two='ttt' YESNO=SSS;
 show create table t1;
 
 --echo #alter table

sql/handler.cc

@@ -4123,6 +4123,7 @@ handler::check_if_supported_inplace_alter(TABLE *altered_table,
     Alter_inplace_info::ALTER_COLUMN_EQUAL_PACK_LENGTH |
     Alter_inplace_info::ALTER_COLUMN_NAME |
     Alter_inplace_info::ALTER_COLUMN_DEFAULT |
+    Alter_inplace_info::ALTER_COLUMN_OPTION |
     Alter_inplace_info::CHANGE_CREATE_OPTION |
     Alter_inplace_info::ALTER_RENAME;
 

sql/handler.h

@@ -1740,7 +1740,10 @@ public:
   // Table is renamed
   static const HA_ALTER_FLAGS ALTER_RENAME               = 1L << 18;
 
-  // Change the storage type of column 
+  // column's engine options changed, something in field->option_struct
+  static const HA_ALTER_FLAGS ALTER_COLUMN_OPTION        = 1L << 19;
+
+  // MySQL alias for the same thing:
   static const HA_ALTER_FLAGS ALTER_COLUMN_STORAGE_TYPE  = 1L << 19;
 
   // Change the column format of column
@@ -1770,7 +1773,7 @@ public:
   // Partition operation with ALL keyword
   static const HA_ALTER_FLAGS ALTER_ALL_PARTITION        = 1L << 28;
 
-  // Partition operation with ALL keyword
+  // Virtual columns changed
   static const HA_ALTER_FLAGS ALTER_COLUMN_VCOL          = 1L << 29;
 
   /**

sql/sql_table.cc

@@ -5894,9 +5894,6 @@ static bool fill_alter_inplace_info(THD *thd,
 
     if (new_field)
     {
-      ha_alter_info->create_info->fields_option_struct[f_ptr - table->field]=
-        new_field->option_struct;
-
       /* Field is not dropped. Evaluate changes bitmap for it. */
 
       /*
@@ -6008,6 +6005,15 @@ static bool fill_alter_inplace_info(THD *thd,
       if (new_field->column_format() != field->column_format())
         ha_alter_info->handler_flags|=
           Alter_inplace_info::ALTER_COLUMN_COLUMN_FORMAT;
+
+      if (engine_options_differ(field->option_struct, new_field->option_struct,
+                                table->file->ht->field_options))
+      {
+        ha_alter_info->handler_flags|= Alter_inplace_info::ALTER_COLUMN_OPTION;
+        ha_alter_info->create_info->fields_option_struct[f_ptr - table->field]=
+          new_field->option_struct;
+      }
+
     }
     else
     {

storage/example/ha_example.cc

@@ -906,38 +906,35 @@ int ha_example::create(const char *name, TABLE *table_arg,
 
 
 /**
-  check_if_incompatible_data() called if ALTER TABLE can't detect otherwise
+  check_if_supported_inplace_alter() is used to ask the engine whether
-  if new and old definition are compatible
+  it can execute this ALTER TABLE statement in place or the server needs to
+  create a new table and copy th data over.
 
-  @details If there are no other explicit signs like changed number of
+  The engine may answer that the inplace alter is not supported or,
-  fields this function will be called by compare_tables()
+  if supported, whether the server should protect the table from concurrent
-  (sql/sql_tables.cc) to decide should we rewrite whole table or only .frm
+  accesses. Return values are
-  file.
 
+    HA_ALTER_INPLACE_NOT_SUPPORTED
+    HA_ALTER_INPLACE_EXCLUSIVE_LOCK
+    HA_ALTER_INPLACE_SHARED_LOCK
+    etc
 */
 
-bool ha_example::check_if_incompatible_data(HA_CREATE_INFO *info,
+enum_alter_inplace_result
-                                            uint table_changes)
+ha_example::check_if_supported_inplace_alter(TABLE* altered_table,
+                                             Alter_inplace_info* ha_alter_info)
+{
+  HA_CREATE_INFO *info= ha_alter_info->create_info;
+  DBUG_ENTER("ha_example::check_if_supported_inplace_alter");
+
+  if (ha_alter_info->handler_flags & Alter_inplace_info::CHANGE_CREATE_OPTION)
   {
-  ha_table_option_struct *param_old, *param_new;
-  DBUG_ENTER("ha_example::check_if_incompatible_data");
     /*
       This example shows how custom engine specific table and field
       options can be accessed from this function to be compared.
     */
-  param_new= info->option_struct;
+    ha_table_option_struct *param_new= info->option_struct;
-  DBUG_PRINT("info", ("new strparam: '%-.64s'  ullparam: %llu  enumparam: %u  "
+    ha_table_option_struct *param_old= table->s->option_struct;
-                      "boolparam: %u",
-                      (param_new->strparam ? param_new->strparam : "<NULL>"),
-                      param_new->ullparam, param_new->enumparam,
-                      param_new->boolparam));
-
-  param_old= table->s->option_struct;
-  DBUG_PRINT("info", ("old strparam: '%-.64s'  ullparam: %llu  enumparam: %u  "
-                      "boolparam: %u",
-                      (param_old->strparam ? param_old->strparam : "<NULL>"),
-                      param_old->ullparam, param_old->enumparam,
-                      param_old->boolparam));
 
     /*
       check important parameters:
@@ -954,7 +951,7 @@ bool ha_example::check_if_incompatible_data(HA_CREATE_INFO *info,
       push_warning_printf(ha_thd(), Sql_condition::WARN_LEVEL_NOTE,
                           ER_UNKNOWN_ERROR, "EXAMPLE DEBUG: ULL %llu -> %llu",
                           param_old->ullparam, param_new->ullparam);
-    DBUG_RETURN(COMPATIBLE_DATA_NO);
+      DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
     }
 
     if (param_new->boolparam != param_old->boolparam)
@@ -962,17 +959,19 @@ bool ha_example::check_if_incompatible_data(HA_CREATE_INFO *info,
       push_warning_printf(ha_thd(), Sql_condition::WARN_LEVEL_NOTE,
                           ER_UNKNOWN_ERROR, "EXAMPLE DEBUG: YESNO %u -> %u",
                           param_old->boolparam, param_new->boolparam);
-    DBUG_RETURN(COMPATIBLE_DATA_NO);
+      DBUG_RETURN(HA_ALTER_INPLACE_NOT_SUPPORTED);
+    }
   }
 
+  if (ha_alter_info->handler_flags & Alter_inplace_info::ALTER_COLUMN_OPTION)
+  {
     for (uint i= 0; i < table->s->fields; i++)
     {
-    ha_field_option_struct *f_old, *f_new;
+      ha_field_option_struct *f_old= table->s->field[i]->option_struct;
-    f_old= table->s->field[i]->option_struct;
+      ha_field_option_struct *f_new= info->fields_option_struct[i];
       DBUG_ASSERT(f_old);
-    if (info->fields_option_struct[i])
+      if (f_new)
       {
-      f_new= info->fields_option_struct[i];
         push_warning_printf(ha_thd(), Sql_condition::WARN_LEVEL_NOTE,
                             ER_UNKNOWN_ERROR, "EXAMPLE DEBUG: Field %`s COMPLEX '%s' -> '%s'",
                             table->s->field[i]->field_name,
@@ -982,8 +981,9 @@ bool ha_example::check_if_incompatible_data(HA_CREATE_INFO *info,
       else
         DBUG_PRINT("info", ("old field %i did not changed", i));
     }
+  }
 
-  DBUG_RETURN(COMPATIBLE_DATA_YES);
+  DBUG_RETURN(HA_ALTER_INPLACE_EXCLUSIVE_LOCK);
 }
 
 

storage/example/ha_example.h

@@ -247,8 +247,9 @@ public:
   int delete_table(const char *from);
   int create(const char *name, TABLE *form,
              HA_CREATE_INFO *create_info);                      ///< required
-  bool check_if_incompatible_data(HA_CREATE_INFO *info,
+  enum_alter_inplace_result
-                                  uint table_changes);
+  check_if_supported_inplace_alter(TABLE* altered_table,
+                                   Alter_inplace_info* ha_alter_info);
 
   THR_LOCK_DATA **store_lock(THD *thd, THR_LOCK_DATA **to,
                              enum thr_lock_type lock_type);     ///< required