MDEV-30984 Online ALTER table is denied with non-informative error me…

…ssages

Group all the checks in online_alter_check_supported().

There is now two groups of checks:
1. A technical availability of online, that is checked before open_tables,
and affects table_list->lock_type. It's supposed to be safe to make it
TL_READ even if COPY algorithm will fall back to not-online, since MDL is
SHARED_UPGRADEABLE anyway.
2. An 'online' availability for a COPY algorithm. It can be done as late as
just before the copy_data_between_tables call. The lock_type influence is
disclosed above, so the only other place it affects is
Alter_info::supports_lock, where `online` flag is only used to decide
whether to report the error at the inplace preparation stage. We'd want to
make that at the last resort, which is COPY preparation, if no algorithm is
chosen by the user. So it's event better now.

Some changes are required to the autoinc support detection, as the check
now happens after mysql_prepare_alter_table:
* alter_info->drop_list is empty
* instead, dropped columns are in tmp_set
* alter_info->create_list now has every field that's in the new table.
* the column definition's change.str will be nonnull whether the column
  remains in the new table (vs whether it was changed, as before).
  But it also has `field` field set.
* IF EXISTS doesn't have to be dealt anymore

This infers that the changes are now checked in more detail: a field's
definition shouldn't be changed, vs a field shouldn't be mentioned in
the CHANGE list, as it was before. This is reflected by the line 193 test.

mysql-test/main/alter_table_online.result

@@ -3,7 +3,7 @@
 #
 create table t (a int);
 alter ignore table t add primary key (a), algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: ALTER IGNORE TABLE. Try LOCK=SHARED
 drop table t;
 #
 # MDEV-28771 Assertion `table->in_use&&tdc->flushed' failed after ALTER
@@ -91,13 +91,13 @@ references t1 (a)
 on update cascade) engine=InnoDB;
 insert into t2 values (1),(2),(3);
 alter table t2 add c int, algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: ON UPDATE CASCADE. Try LOCK=SHARED
 alter table t2 add c int, algorithm=inplace, lock=none;
 create or replace table t2 (b int, foreign key (b)
 references t1 (a)
 on delete set null) engine=InnoDB;
 alter table t2 add c int, algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: ON DELETE SET NULL. Try LOCK=SHARED
 alter table t2 add c int, algorithm=inplace, lock=none;
 create or replace table t2 (b int, foreign key (b)
 references t1 (a)
@@ -116,7 +116,7 @@ create table t2 (a int references t1 (a),
 b int references t1 (b) on update cascade) engine=InnoDB;
 insert into t2 values (1, 1),(2, 2);
 alter table t2 add c int, algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: ON UPDATE CASCADE. Try LOCK=SHARED
 alter table t2 add c int, algorithm=copy;
 alter table t2 add d int, algorithm=inplace;
 drop table t2, t1;
@@ -131,7 +131,7 @@ row_end bigint unsigned generated always as row end,
 period for system_time (row_start, row_end))
 engine=innodb with system versioning;
 alter table t1 add c int, algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: BIGINT GENERATED ALWAYS AS ROW_START. Try LOCK=SHARED
 alter table t1 add c int, algorithm=inplace;
 alter table t1 add d int, lock=none;
 set system_versioning_alter_history= default;
@@ -142,7 +142,7 @@ drop table t1;
 #
 create table t (a serial, b int) engine=innodb;
 alter table t drop a, modify b serial, algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED
 set statement sql_mode= NO_AUTO_VALUE_ON_ZERO for
 alter table t drop a, modify b serial, algorithm=copy, lock=none;
 create or replace table t (a serial, b int) engine=innodb;
@@ -156,7 +156,7 @@ t	CREATE TABLE `t` (
 # a is unique in the old table, but is shrunk in the new one.
 # Only unsafe approach is fine because of possible collisions.
 alter table t modify a int, modify b serial, algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED
 #
 # Check that we treat autoinc columns correctly modify old autoinc is
 # fine, adding new autoinc for existed column is unsafe.
@@ -179,25 +179,27 @@ t	CREATE TABLE `t` (
   UNIQUE KEY `b` (`b`)
 ) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci
 alter table t drop b, change c c serial, algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED
 # Check existed unique keys.
 create or replace table t(a int, b int not null, c int not null, d int);
 # No unique in the old table;
 alter table t add unique(b, c), modify d int auto_increment, add key(d),
 algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED
 alter table t add unique(a, b);
 # Unique in the old table has nulls;
 alter table t modify d int auto_increment, add key(d),
 algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED
 alter table t add unique(b, c);
-# Change unique'scolumn;
-alter table t change b x int, modify d int auto_increment, add key(d),
+# Change unique's column;
+alter table t change b x bigint, modify d int auto_increment, add key(d),
 algorithm=copy, lock=none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
-# Finally good.
-alter table t modify d int auto_increment, add key(d), 
+ERROR 0A000: LOCK=NONE is not supported. Reason: CHANGE COLUMN ... AUTO_INCREMENT. Try LOCK=SHARED
+# Finally good. Simple renames with a type unchanged will not affect
+# the result. Also NOT NULL -> NULL transform is fine.
+alter table t modify d int auto_increment, add key(d),
+change b x int null,
 algorithm=copy, lock=none;
 drop table t;
 # MDEV-31172 Server crash or ASAN errors in online_alter_check_autoinc

mysql-test/main/alter_table_online.test

@@ -190,13 +190,15 @@ alter table t modify d int auto_increment, add key(d),
               algorithm=copy, lock=none;
 
 alter table t add unique(b, c);
---echo # Change unique'scolumn;
+--echo # Change unique's column;
 --error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
-alter table t change b x int, modify d int auto_increment, add key(d),
+alter table t change b x bigint, modify d int auto_increment, add key(d),
               algorithm=copy, lock=none;
 
---echo # Finally good.
-alter table t modify d int auto_increment, add key(d), 
+--echo # Finally good. Simple renames with a type unchanged will not affect
+--echo # the result. Also NOT NULL -> NULL transform is fine.
+alter table t modify d int auto_increment, add key(d),
+              change b x int null,
               algorithm=copy, lock=none;
 
 drop table t;

mysql-test/main/alter_table_online_debug.result

@@ -491,7 +491,7 @@ a	b	UNIX_TIMESTAMP(row_start)	UNIX_TIMESTAMP(row_end)
 3	44	1.000000	2147483647.999999
 6	77	1.000000	2147483647.999999
 alter table t1 drop system versioning, algorithm= copy, lock= none;
-ERROR 0A000: LOCK=NONE is not supported. Reason: COPY algorithm requires a lock. Try LOCK=SHARED
+ERROR 0A000: LOCK=NONE is not supported. Reason: DROP SYSTEM VERSIONING. Try LOCK=SHARED
 #
 # Test ROLLBACK TO SAVEPOINT
 #

sql/sql_class.h

@@ -1233,6 +1233,16 @@ class Query_arena
   { return strdup_root(mem_root,str); }
   inline char *strmake(const char *str, size_t size) const
   { return strmake_root(mem_root,str,size); }
+  inline LEX_CSTRING strcat(const LEX_CSTRING &a, const LEX_CSTRING &b) const
+  {
+    char *buf= (char*)alloc(a.length + b.length + 1);
+    if (unlikely(!buf))
+      return null_clex_str;
+    memcpy(buf, a.str, a.length);
+    memcpy(buf + a.length, b.str, b.length);
+    buf[a.length + b.length] = 0;
+    return {buf, a.length + b.length};
+  }
   inline void *memdup(const void *str, size_t size) const
   { return memdup_root(mem_root,str,size); }
   inline void *memdup_w_gap(const void *str, size_t size, size_t gap) const

sql/sql_table.cc

@@ -9871,52 +9871,89 @@ bool online_alter_check_autoinc(const THD *thd, const Alter_info *alter_info,
   */
   for (uint k= 0; k < table->s->keys; k++)
   {
-    const KEY &key= table->s->key_info[k];
+    const KEY &key= table->key_info[k];
     if ((key.flags & HA_NOSAME) == 0 || key.flags & HA_NULL_PART_KEY)
       continue;
     bool key_parts_good= true;
     for (uint kp= 0; kp < key.user_defined_key_parts && key_parts_good; kp++)
     {
-      const char *field_name= key.key_part[kp].field->field_name.str;
-      for (const auto &c: alter_info->drop_list)
-        if (c.type == Alter_drop::COLUMN
-            && my_strcasecmp(system_charset_info, c.name, field_name) == 0)
-        {
-          key_parts_good= false;
-          break;
-        }
+      const Field *f= key.key_part[kp].field;
+      // tmp_set contains dropped fields after mysql_prepare_alter_table
+      key_parts_good= !bitmap_is_set(&table->tmp_set, f->field_index);
+
       if (key_parts_good)
         for (const auto &c: alter_info->create_list)
-          if (c.change.str && my_strcasecmp(system_charset_info, c.change.str,
-                                            field_name) == 0)
+          if (c.field == f)
           {
-            key_parts_good= false;
+            key_parts_good= f->is_equal(c);
             break;
           }
     }
     if (key_parts_good)
       return true;
   }
 
-  const char *old_autoinc= table->found_next_number_field
-                           ? table->found_next_number_field->field_name.str
-                           : "";
-  bool online= true;
   for (const auto &c: alter_info->create_list)
   {
-    if (c.change.str && c.flags & AUTO_INCREMENT_FLAG)
+    if (c.flags & AUTO_INCREMENT_FLAG)
+    {
+      if (c.field && !(c.field->flags & AUTO_INCREMENT_FLAG))
+        return false;
+      break;
+    }
+  }
+  return true;
+}
+
+static
+const char *online_alter_check_supported(const THD *thd,
+                                         const Alter_info *alter_info,
+                                         const TABLE *table, bool *online)
+{
+  DBUG_ASSERT(*online);
+
+  *online= thd->locked_tables_mode != LTM_LOCK_TABLES && !table->s->tmp_table;
+  if (!*online)
+    return NULL;
+
+  *online= (alter_info->flags & ALTER_DROP_SYSTEM_VERSIONING) == 0;
+  if (!*online)
+    return "DROP SYSTEM VERSIONING";
+
+  *online= !thd->lex->ignore;
+  if (!*online)
+    return "ALTER IGNORE TABLE";
+
+  *online= !table->versioned(VERS_TRX_ID);
+  if (!*online)
+    return "BIGINT GENERATED ALWAYS AS ROW_START";
+
+  List<FOREIGN_KEY_INFO> fk_list;
+  table->file->get_foreign_key_list(thd, &fk_list);
+  for (auto &fk: fk_list)
+  {
+    if (fk_modifies_child(fk.delete_method) ||
+        fk_modifies_child(fk.update_method))
     {
-      if (my_strcasecmp(system_charset_info, c.change.str,  old_autoinc) != 0)
+      *online= false;
+      // Don't fall to a common unsupported case to avoid heavy string ops.
+      if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_NONE)
       {
-        if (c.create_if_not_exists // check IF EXISTS option
-            && table->find_field_by_name(&c.change) == NULL)
-          continue;
-        online= false;
+        return fk_modifies_child(fk.delete_method)
+               ? thd->strcat({STRING_WITH_LEN("ON DELETE ")},
+                             *fk_option_name(fk.delete_method)).str
+               : thd->strcat({STRING_WITH_LEN("ON UPDATE ")},
+                             *fk_option_name(fk.update_method)).str;
       }
-      break;
+      return NULL;
     }
   }
-  return online;
+
+  *online= online_alter_check_autoinc(thd, alter_info, table);
+  if (!*online)
+    return "CHANGE COLUMN ... AUTO_INCREMENT";
+
+  return NULL;
 }
 
 
@@ -10100,9 +10137,7 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db,
   table_list->required_type= TABLE_TYPE_NORMAL;
 
   if (alter_info->requested_lock > Alter_info::ALTER_TABLE_LOCK_NONE
-      || alter_info->flags & ALTER_DROP_SYSTEM_VERSIONING
       || thd->lex->sql_command == SQLCOM_OPTIMIZE
-      || ignore
       || alter_info->algorithm(thd) > Alter_info::ALTER_TABLE_ALGORITHM_COPY)
     online= false;
 
@@ -10142,22 +10177,6 @@ bool mysql_alter_table(THD *thd, const LEX_CSTRING *new_db,
   table= table_list->table;
   bool is_reg_table= table->s->tmp_table == NO_TMP_TABLE;
 
-  online= online && !table->s->tmp_table && !table->versioned(VERS_TRX_ID);
-
-  List<FOREIGN_KEY_INFO> fk_list;
-  table->file->get_foreign_key_list(thd, &fk_list);
-  for (auto &fk: fk_list)
-  {
-    if (fk_modifies_child(fk.delete_method)
-        || fk_modifies_child(fk.update_method))
-    {
-      online= false;
-      break;
-    }
-  }
-
-  online= online && online_alter_check_autoinc(thd, alter_info, table);
-
 #ifdef WITH_WSREP
   if (WSREP(thd) &&
       (thd->lex->sql_command == SQLCOM_ALTER_TABLE ||
@@ -10979,22 +10998,22 @@ do_continue:;
   if (fk_prepare_copy_alter_table(thd, table, alter_info, &alter_ctx))
     goto err_new_table_cleanup;
 
-  if (!table->s->tmp_table)
+  if (online)
   {
-    // COPY algorithm doesn't work with concurrent writes.
-    if (!online &&
+    const char *reason= online_alter_check_supported(thd, alter_info, table,
+                                                     &online);
+    if (reason &&
         alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_NONE)
     {
+      DBUG_ASSERT(!online);
       my_error(ER_ALTER_OPERATION_NOT_SUPPORTED_REASON, MYF(0),
-               "LOCK=NONE",
-               ER_THD(thd, ER_ALTER_OPERATION_NOT_SUPPORTED_REASON_COPY),
-               "LOCK=SHARED");
+               "LOCK=NONE", reason, "LOCK=SHARED");
       goto err_new_table_cleanup;
     }
+  }
 
-    if (thd->locked_tables_mode == LTM_LOCK_TABLES)
-      online= false;
-
+  if (!table->s->tmp_table)
+  {
     // If EXCLUSIVE lock is requested, upgrade already.
     if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE &&
         wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))