BUG#26881 - Large MERGE tables report incorrect specification when no

            differences in tables
Certain merge tables were wrongly reported as having incorrect definition:
- Some fields that are 1 byte long (e.g. TINYINT, CHAR(1)), might
  be internally casted (in certain cases) to a different type on a
  storage engine layer. (affects 4.1 and up)
- If tables in a merge (and a MERGE table itself) had short VARCHAR column (less
  than 4 bytes) and at least one (but not all) tables were ALTER'ed (even to an
  identical table: ALTER TABLE xxx ENGINE=yyy), table definitions went ouf of
  sync. (affects 4.1 only)

This is fixed by relaxing a check for underlying conformance and setting
field type to FIELD_TYPE_STRING in case varchar is shorter than 4
when a table is created.


myisam/mi_create.c:
  Added a comment.
mysql-test/r/merge.result:
  A test case for bug#26881.
mysql-test/t/merge.test:
  A test case for bug#26881.
sql/ha_myisam.cc:
  Relaxed some checks performed by check_definition():
  As comparing of fulltext keys (and key segments) is not yet implemented,
  only return an error in case one of keys is fulltext and other is not.
  Otherwise, if both keys are fulltext, accept them as is.
  
  As comparing of spatial keys (and key segments) is not yet implemented,
  only return an error in case one of keys is spatial and other is not.
  Otherwise, if both keys are spatial, accept them as is.
  
  A workaround to handle situation when field is casted from FIELD_SKIP_ZERO
  to FIELD_NORMAL. This could happen only in case field length is 1 and row
  format is fixed.
sql/sql_parse.cc:
  When a table that has varchar field shorter than 4 is created, field type is
  set to FIELD_TYPE_VAR_STRING. Later, when a table is modified using alter
  table, field type is changed to FIELD_TYPE_STRING (see Field_string::type).
  That means HA_OPTION_PACK_RECORD flag might be lost and thus null_bit might
  be shifted by alter table, in other words alter table doesn't create 100%
  equal table definition.
  
  This is usually not a problem, since when a table is created/altered,
  definition on a storage engine layer is based on one that is passed from
  sql layer. But it is a problem for merge engine - null_bit is shifted when
  a table (merge or underlying) is altered.
  
  Set field type to FIELD_TYPE_STRING in case FIELD_TYPE_VAR_STRING is shorter
  than 4 when a table is created as it is done in Field::type.

myisam/mi_create.c

@@ -158,6 +158,10 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
       rec--;
       if (rec->type == (int) FIELD_SKIP_ZERO && rec->length == 1)
       {
+        /*
+          NOTE1: here we change a field type FIELD_SKIP_ZERO ->
+          FIELD_NORMAL
+        */
 	rec->type=(int) FIELD_NORMAL;
 	packed--;
 	min_pack_length++;

mysql-test/r/merge.result

@@ -806,3 +806,16 @@ CREATE TABLE t2(c1 INT) ENGINE=MERGE UNION=(t1);
 INSERT DELAYED INTO t2 VALUES(1);
 ERROR HY000: Table storage engine for 't2' doesn't have this option
 DROP TABLE t1, t2;
+CREATE TABLE t1(c1 VARCHAR(1));
+CREATE TABLE m1 LIKE t1;
+ALTER TABLE m1 ENGINE=MERGE UNION=(t1);
+SELECT * FROM m1;
+c1
+DROP TABLE t1, m1;
+CREATE TABLE t1(c1 VARCHAR(4), c2 TINYINT, c3 TINYINT, c4 TINYINT,
+c5 TINYINT, c6 TINYINT, c7 TINYINT, c8 TINYINT, c9 TINYINT);
+CREATE TABLE m1 LIKE t1;
+ALTER TABLE m1 ENGINE=MERGE UNION=(t1);
+SELECT * FROM m1;
+c1	c2	c3	c4	c5	c6	c7	c8	c9
+DROP TABLE t1, m1;

mysql-test/t/merge.test

@@ -437,4 +437,21 @@ CREATE TABLE t2(c1 INT) ENGINE=MERGE UNION=(t1);
 INSERT DELAYED INTO t2 VALUES(1);
 DROP TABLE t1, t2;
 
+#
+# BUG#26881 - Large MERGE tables report incorrect specification when no
+#             differences in tables
+#
+CREATE TABLE t1(c1 VARCHAR(1));
+CREATE TABLE m1 LIKE t1;
+ALTER TABLE m1 ENGINE=MERGE UNION=(t1);
+SELECT * FROM m1;
+DROP TABLE t1, m1;
+
+CREATE TABLE t1(c1 VARCHAR(4), c2 TINYINT, c3 TINYINT, c4 TINYINT,
+                c5 TINYINT, c6 TINYINT, c7 TINYINT, c8 TINYINT, c9 TINYINT);
+CREATE TABLE m1 LIKE t1;
+ALTER TABLE m1 ENGINE=MERGE UNION=(t1);
+SELECT * FROM m1;
+DROP TABLE t1, m1;
+
 # End of 4.1 tests

sql/ha_myisam.cc

@@ -304,6 +304,12 @@ int table2myisam(TABLE *table_arg, MI_KEYDEF **keydef_out,
   RETURN VALUE
     0 - Equal definitions.
     1 - Different definitions.
+
+  TODO
+    - compare FULLTEXT keys;
+    - compare SPATIAL keys;
+    - compare FIELD_SKIP_ZERO which is converted to FIELD_NORMAL correctly
+      (should be corretly detected in table2myisam).
 */
 
 int check_definition(MI_KEYDEF *t1_keyinfo, MI_COLUMNDEF *t1_recinfo,
@@ -329,6 +335,28 @@ int check_definition(MI_KEYDEF *t1_keyinfo, MI_COLUMNDEF *t1_recinfo,
   {
     HA_KEYSEG *t1_keysegs= t1_keyinfo[i].seg;
     HA_KEYSEG *t2_keysegs= t2_keyinfo[i].seg;
+    if (t1_keyinfo[i].flag & HA_FULLTEXT && t2_keyinfo[i].flag & HA_FULLTEXT)
+      continue;
+    else if (t1_keyinfo[i].flag & HA_FULLTEXT ||
+             t2_keyinfo[i].flag & HA_FULLTEXT)
+    {
+       DBUG_PRINT("error", ("Key %d has different definition", i));
+       DBUG_PRINT("error", ("t1_fulltext= %d, t2_fulltext=%d",
+                            test(t1_keyinfo[i].flag & HA_FULLTEXT),
+                            test(t2_keyinfo[i].flag & HA_FULLTEXT)));
+       DBUG_RETURN(1);
+    }
+    if (t1_keyinfo[i].flag & HA_SPATIAL && t2_keyinfo[i].flag & HA_SPATIAL)
+      continue;
+    else if (t1_keyinfo[i].flag & HA_SPATIAL ||
+             t2_keyinfo[i].flag & HA_SPATIAL)
+    {
+       DBUG_PRINT("error", ("Key %d has different definition", i));
+       DBUG_PRINT("error", ("t1_spatial= %d, t2_spatial=%d",
+                            test(t1_keyinfo[i].flag & HA_SPATIAL),
+                            test(t2_keyinfo[i].flag & HA_SPATIAL)));
+       DBUG_RETURN(1);
+    }
     if (t1_keyinfo[i].keysegs != t2_keyinfo[i].keysegs ||
         t1_keyinfo[i].key_alg != t2_keyinfo[i].key_alg)
     {
@@ -365,7 +393,14 @@ int check_definition(MI_KEYDEF *t1_keyinfo, MI_COLUMNDEF *t1_recinfo,
   {
     MI_COLUMNDEF *t1_rec= &t1_recinfo[i];
     MI_COLUMNDEF *t2_rec= &t2_recinfo[i];
-    if (t1_rec->type != t2_rec->type ||
+    /*
+      FIELD_SKIP_ZERO can be changed to FIELD_NORMAL in mi_create,
+      see NOTE1 in mi_create.c
+    */
+    if ((t1_rec->type != t2_rec->type &&
+         !(t1_rec->type == (int) FIELD_SKIP_ZERO &&
+           t1_rec->length == 1 &&
+           t2_rec->type == (int) FIELD_NORMAL)) ||
         t1_rec->length != t2_rec->length ||
         t1_rec->null_bit != t2_rec->null_bit)
     {

sql/sql_parse.cc

@@ -4653,8 +4653,14 @@ bool add_field_to_list(THD *thd, char *field_name, enum_field_types type,
 	new_field->length++;
     }
     break;
-  case FIELD_TYPE_STRING:
   case FIELD_TYPE_VAR_STRING:
+    if (new_field->length < 4)
+    {
+      new_field->sql_type= FIELD_TYPE_STRING;
+      break;
+    }
+    /* fall through */
+  case FIELD_TYPE_STRING:
     if (new_field->length <= MAX_FIELD_CHARLENGTH || default_value)
       break;
     /* Convert long CHAR() and VARCHAR columns to TEXT or BLOB */