Bug#30943642: ERROR 1846 (0A000) AT LINE 1: ALGORITHM=INPLACE IS NOT SUPPORTED. REASON: CANNO

Problem: ALTER on a table with a SET column which had a display width >
255 would not be done INPLACE, even if otherwise possible.

Solution: Fix incorrect inplace check for SETs (display width is
irrelevant for SETS (and enums) since this does not affect the amount
storage needed.

Change-Id: Ide8743047de62f87ed5e2cf268f447d4c93d6470

Author: Dyre Tjeldvoll

mysql-test/r/alter_table.result

@@ -4425,3 +4425,37 @@ CREATE TABLE t1 (b INT);
 # The below ALTER TABLE causes assertion failure without the fix.
 ALTER TABLE t1 ADD gcol INT AS (b + 1) VIRTUAL FIRST, ADD COLUMN a INT;
 DROP TABLE t1;
+#
+# Bug#30943642 ERROR 1846 (0A000) AT LINE 1: ALGORITHM=INPLACE IS NOT
+# SUPPORTED. REASON: CANNO
+#
+CREATE TABLE t1 (c1
+SET('1234567890123456789a','1234567890123456789b','12345678901234567890c'));
+ALTER TABLE t1 ADD c2 INT NULL AFTER c1, ALGORITHM=INPLACE;
+DROP TABLE t1;
+CREATE TABLE t1 (c1
+SET('1234567890123456789a','1234567890123456789b','12345678901234567890cd')
+DEFAULT NULL);
+# Verify that it is possible to add column to table with set column
+# having display width in bytes >= 256
+ALTER TABLE t1 ADD c2 INT NULL AFTER c1, ALGORITHM=INPLACE;
+DROP TABLE t1;
+# Enums are not affected, because in order for display width in bytes
+# to exceed 255 we need an invidual enum value with this this size,
+# and this is not permitted.
+CREATE TABLE t1 (c1 ENUM ('000', '001',
+'0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'));
+ERROR HY000: Too long enumeration/set value for column c1.
+CREATE TABLE t1 (c1 SET ('00', '01', '02', '03', '04', '05', '06'));
+# Verify that a SET change which requires more space still cannot be
+# done inplace
+ALTER TABLE t1 MODIFY COLUMN c1 SET ('00', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15'), ALGORITHM=INPLACE;
+ERROR 0A000: ALGORITHM=INPLACE is not supported. Reason: Cannot change column type INPLACE. Try ALGORITHM=COPY.
+DROP TABLE t1;
+CREATE TABLE t1 (c1 ENUM ('000', '001', '002', '003'));
+# Verify that an ENUM change which requires more space still cannot be
+# done inplace
+ALTER TABLE t1 MODIFY COLUMN c1 ENUM (
+'000', '001', '002', '003', '004', '005', '006', '007', '008', '009', '010', '011', '012', '013', '014', '015', '016', '017', '018', '019', '020', '021', '022', '023', '024', '025', '026', '027', '028', '029', '030', '031', '032', '033', '034', '035', '036', '037', '038', '039', '040', '041', '042', '043', '044', '045', '046', '047', '048', '049', '050', '051', '052', '053', '054', '055', '056', '057', '058', '059', '060', '061', '062', '063', '064', '065', '066', '067', '068', '069', '070', '071', '072', '073', '074', '075', '076', '077', '078', '079', '080', '081', '082', '083', '084', '085', '086', '087', '088', '089', '090', '091', '092', '093', '094', '095', '096', '097', '098', '099', '100', '101', '102', '103', '104', '105', '106', '107', '108', '109', '110', '111', '112', '113', '114', '115', '116', '117', '118', '119', '120', '121', '122', '123', '124', '125', '126', '127', '128', '129', '130', '131', '132', '133', '134', '135', '136', '137', '138', '139', '140', '141', '142', '143', '144', '145', '146', '147', '148', '149', '150', '151', '152', '153', '154', '155', '156', '157', '158', '159', '160', '161', '162', '163', '164', '165', '166', '167', '168', '169', '170', '171', '172', '173', '174', '175', '176', '177', '178', '179', '180', '181', '182', '183', '184', '185', '186', '187', '188', '189', '190', '191', '192', '193', '194', '195', '196', '197', '198', '199', '200', '201', '202', '203', '204', '205', '206', '207', '208', '209', '210', '211', '212', '213', '214', '215', '216', '217', '218', '219', '220', '221', '222', '223', '224', '225', '226', '227', '228', '229', '230', '231', '232', '233', '234', '235', '236', '237', '238', '239', '240', '241', '242', '243', '244', '245', '246', '247', '248', '249', '250', '251', '252', '253', '254', '255', '256'), ALGORITHM=INPLACE;
+ERROR 0A000: ALGORITHM=INPLACE is not supported. Reason: Cannot change column type INPLACE. Try ALGORITHM=COPY.
+DROP TABLE t1;

mysql-test/t/alter_table.test

@@ -3546,3 +3546,47 @@ CREATE TABLE t1 (b INT);
 ALTER TABLE t1 ADD gcol INT AS (b + 1) VIRTUAL FIRST, ADD COLUMN a INT;
 DROP TABLE t1;
 
+
+--echo #
+--echo # Bug#30943642 ERROR 1846 (0A000) AT LINE 1: ALGORITHM=INPLACE IS NOT
+--echo # SUPPORTED. REASON: CANNO
+--echo #
+
+CREATE TABLE t1 (c1
+SET('1234567890123456789a','1234567890123456789b','12345678901234567890c'));
+ALTER TABLE t1 ADD c2 INT NULL AFTER c1, ALGORITHM=INPLACE;
+DROP TABLE t1;
+
+CREATE TABLE t1 (c1
+SET('1234567890123456789a','1234567890123456789b','12345678901234567890cd')
+DEFAULT NULL);
+
+--echo # Verify that it is possible to add column to table with set column
+--echo # having display width in bytes >= 256
+ALTER TABLE t1 ADD c2 INT NULL AFTER c1, ALGORITHM=INPLACE;
+DROP TABLE t1;
+
+--echo # Enums are not affected, because in order for display width in bytes
+--echo # to exceed 255 we need an invidual enum value with this this size,
+--echo # and this is not permitted.
+--error ER_TOO_LONG_SET_ENUM_VALUE
+CREATE TABLE t1 (c1 ENUM ('000', '001',
+'0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'));
+
+CREATE TABLE t1 (c1 SET ('00', '01', '02', '03', '04', '05', '06'));
+
+--echo # Verify that a SET change which requires more space still cannot be
+--echo # done inplace
+--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
+ALTER TABLE t1 MODIFY COLUMN c1 SET ('00', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15'), ALGORITHM=INPLACE;
+DROP TABLE t1;
+
+CREATE TABLE t1 (c1 ENUM ('000', '001', '002', '003'));
+
+--echo # Verify that an ENUM change which requires more space still cannot be
+--echo # done inplace
+--error ER_ALTER_OPERATION_NOT_SUPPORTED_REASON
+ALTER TABLE t1 MODIFY COLUMN c1 ENUM (
+'000', '001', '002', '003', '004', '005', '006', '007', '008', '009', '010', '011', '012', '013', '014', '015', '016', '017', '018', '019', '020', '021', '022', '023', '024', '025', '026', '027', '028', '029', '030', '031', '032', '033', '034', '035', '036', '037', '038', '039', '040', '041', '042', '043', '044', '045', '046', '047', '048', '049', '050', '051', '052', '053', '054', '055', '056', '057', '058', '059', '060', '061', '062', '063', '064', '065', '066', '067', '068', '069', '070', '071', '072', '073', '074', '075', '076', '077', '078', '079', '080', '081', '082', '083', '084', '085', '086', '087', '088', '089', '090', '091', '092', '093', '094', '095', '096', '097', '098', '099', '100', '101', '102', '103', '104', '105', '106', '107', '108', '109', '110', '111', '112', '113', '114', '115', '116', '117', '118', '119', '120', '121', '122', '123', '124', '125', '126', '127', '128', '129', '130', '131', '132', '133', '134', '135', '136', '137', '138', '139', '140', '141', '142', '143', '144', '145', '146', '147', '148', '149', '150', '151', '152', '153', '154', '155', '156', '157', '158', '159', '160', '161', '162', '163', '164', '165', '166', '167', '168', '169', '170', '171', '172', '173', '174', '175', '176', '177', '178', '179', '180', '181', '182', '183', '184', '185', '186', '187', '188', '189', '190', '191', '192', '193', '194', '195', '196', '197', '198', '199', '200', '201', '202', '203', '204', '205', '206', '207', '208', '209', '210', '211', '212', '213', '214', '215', '216', '217', '218', '219', '220', '221', '222', '223', '224', '225', '226', '227', '228', '229', '230', '231', '232', '233', '234', '235', '236', '237', '238', '239', '240', '241', '242', '243', '244', '245', '246', '247', '248', '249', '250', '251', '252', '253', '254', '255', '256'), ALGORITHM=INPLACE;
+DROP TABLE t1;
+

sql/field.cc

@@ -132,6 +132,7 @@ namespace {
    condition, false otherwise.
 */
 bool sql_type_prevents_inplace(const Field &from, const Create_field &to) {
+  DBUG_TRACE;
   return to.sql_type != from.real_type();
 }
 
@@ -147,19 +148,35 @@ bool sql_type_prevents_inplace(const Field &from, const Create_field &to) {
    condition, false otherwise.
 */
 bool length_prevents_inplace(const Field &from, const Create_field &to) {
-  DBUG_PRINT("inplace", ("stmt: %s", current_thd->query().str));
-
-  DBUG_PRINT("inplace", ("from: field_length:%u, pack_length():%u, "
-                         "row_pack_length():%u, max_display_length():%u",
-                         from.field_length, from.pack_length(),
-                         from.row_pack_length(), from.max_display_length()));
-
-  DBUG_PRINT("inplace", ("to: length:%zu, pack_length:%zu",
-                         to.max_display_width_in_bytes(), to.pack_length()));
+  DBUG_TRACE;
+  DBUG_PRINT(
+      "inplace",
+      ("from:%p, to.field:%p, to.field->row_pack_length():%u, "
+       "to.max_display_width_in_bytes():%zu",
+       &from, to.field, to.field ? to.field->row_pack_length() : (uint)-1,
+       to.max_display_width_in_bytes()));
+
+  if (to.pack_length() < from.pack_length()) {
+    DBUG_PRINT(
+        "inplace",
+        ("decreasing pack_length from %u to %zu, -> true for '%s'",
+         from.pack_length(), to.pack_length(), current_thd->query().str));
+    return true;
+  }
 
-  return (
-      to.pack_length() < from.pack_length() ||
-      (to.max_display_width_in_bytes() >= 256 && from.row_pack_length() < 256));
+  if (to.max_display_width_in_bytes() >= 256 && from.row_pack_length() < 256) {
+    DBUG_PRINT("inplace",
+               ("row_pack_length increases past the 256 threshold, from %u to "
+                "%zu, -> true for '%s'",
+                from.row_pack_length(), to.max_display_width_in_bytes(),
+                current_thd->query().str));
+    DBUG_PRINT("inplace",
+               ("from:%p, to.field:%p, to.field->row_pack_length():%u", &from,
+                to.field, to.field ? to.field->row_pack_length() : (uint)-1));
+    return true;
+  }
+  DBUG_PRINT("inplace", ("-> false"));
+  return false;
 }
 
 /**
@@ -188,6 +205,8 @@ bool length_prevents_inplace(const Field &from, const Create_field &to) {
    condition, false otherwise.
 */
 bool charset_prevents_inplace(const Field_str &from, const Create_field &to) {
+  DBUG_TRACE;
+
   if (my_charset_same(to.charset, from.charset()) ||
       my_charset_same(to.charset, &my_charset_bin)) {
     return false;
@@ -208,6 +227,7 @@ bool charset_prevents_inplace(const Field_str &from, const Create_field &to) {
    condition, false otherwise.
 */
 bool change_prevents_inplace(const Field_str &from, const Create_field &to) {
+  DBUG_TRACE;
   return sql_type_prevents_inplace(from, to) ||
          length_prevents_inplace(from, to) ||
          charset_prevents_inplace(from, to);
@@ -6051,6 +6071,8 @@ bool Field::gcol_expr_is_equal(const Create_field *field) const {
 }
 
 uint Field_str::is_equal(const Create_field *new_field) const {
+  DBUG_TRACE;
+
   if (change_prevents_inplace(*this, *new_field)) {
     return IS_EQUAL_NO;
   }
@@ -6745,6 +6767,7 @@ Field *Field_varstring::new_key_field(MEM_ROOT *root, TABLE *new_table,
 }
 
 uint Field_varstring::is_equal(const Create_field *new_field) const {
+  DBUG_TRACE;
   if (change_prevents_inplace(*this, *new_field)) {
     return IS_EQUAL_NO;
   }
@@ -7281,6 +7304,8 @@ uint Field_blob::max_packed_col_length() const {
 }
 
 uint Field_blob::is_equal(const Create_field *new_field) const {
+  DBUG_TRACE;
+
   // Can't use change_prevents_inplace() here as it uses
   // sql_type_prevents_inplace() which checks real_type(), and
   // Field_blob::real_type() does NOT return the actual blob type as
@@ -8299,17 +8324,15 @@ bool Field_enum::eq_def(const Field *field) const {
 */
 
 uint Field_enum::is_equal(const Create_field *new_field) const {
+  DBUG_TRACE;
   /*
     The fields are compatible if they have the same flags,
     type, charset and have the same underlying length.
   */
-  if (change_prevents_inplace(*this, *new_field)) {
-    DBUG_PRINT("inplace", ("change_prevents_inplace() -> IS_EQUAL_NO"));
+  if (sql_type_prevents_inplace(*this, *new_field)) {
+    DBUG_PRINT("inplace", ("-> IS_EQUAL_NO"));
     return IS_EQUAL_NO;
   }
-  // Assert - either the charset is unchanged or the change is
-  // inplace-compatible.
-
   TYPELIB *new_typelib = new_field->interval;
 
   /*

sql/sql_alter.h

@@ -326,7 +326,7 @@ class Alter_info {
 
     /// Set if ANY engine attribute is used (also in CREATE) Note that
     /// this is NOT to be set for SECONDARY_ENGINE_ATTRIBUTE as this flag
-    /// controls if execution schould check if SE supports engine
+    /// controls if execution should check if SE supports engine
     /// attributes.
     ANY_ENGINE_ATTRIBUTE = 1ULL << 39
   };

sql/sql_table.cc

@@ -11234,6 +11234,8 @@ static bool fill_alter_inplace_info(THD *thd, TABLE *table,
   */
   uint old_field_index_without_vgc = 0;
   for (f_ptr = table->field; (field = *f_ptr); f_ptr++) {
+    DBUG_PRINT("inplace", ("Existing field: %s", field->field_name));
+
     /* Clear marker for renamed or dropped field
     which we are going to set later. */
     field->clear_flag(FIELD_IS_RENAMED);
@@ -11257,6 +11259,8 @@ static bool fill_alter_inplace_info(THD *thd, TABLE *table,
       */
       switch (field->is_equal(new_field)) {
         case IS_EQUAL_NO:
+          DBUG_PRINT("inplace", ("Field %s: IS_EQUAL_NO for '%s'",
+                                 field->field_name, thd->query().str));
           /* New column type is incompatible with old one. */
           if (field->is_virtual_gcol())
             ha_alter_info->handler_flags |=