MDEV-20494 ER_NOT_FORM_FILE or assertion upon adding partition to period table

- Fixed mysql_prepare_create_table() constraint duplicate checking;
- Refactored period constraint handling in mysql_prepare_alter_table():
  * No need to allocate new objects;
  * Keep old constraint name but exclude it from dup checking by automatic_name;
- Some minor memory leaks fixed;
- Some conceptual TODOs.

mysql-test/suite/period/r/alter.result

@@ -174,3 +174,19 @@ alter table t add constraint mytime_1 check (x > 2);
 insert t values (3, @e, @s);
 ERROR 23000: CONSTRAINT `mytime_2` failed for `test`.`t`
 drop table t;
+#
+# MDEV-20494 ER_NOT_FORM_FILE or assertion upon adding partition to period table
+#
+create table t1 (f date, t date, period for app(f,t)) with system versioning partition by system_time ( partition p1 history, partition pn current );
+lock table t1 write;
+alter table t1 add partition (partition p2 history);
+Warnings:
+Warning	4115	Maybe missing parameters: no rotation condition for multiple HISTORY partitions.
+unlock tables;
+create or replace table t1 (x int, s date, e date, period for app(s,e));
+insert into t1 values(1, '2020-03-01', '2020-03-02');
+insert into t1 values(1, '2020-03-01', '2020-03-02');
+alter table t1 add primary key(x, s, e);
+ERROR 23000: Duplicate entry '1-2020-03-01-2020-03-02' for key 'PRIMARY'
+alter table t1 add system versioning;
+drop table t1;

mysql-test/suite/period/t/alter.test

+--source include/have_partition.inc
+
 set @s= '1992-01-01';
 set @e= '1999-12-31';
 
@@ -131,3 +133,21 @@ alter table t add constraint mytime_1 check (x > 2);
 insert t values (3, @e, @s);
 
 drop table t;
+
+--echo #
+--echo # MDEV-20494 ER_NOT_FORM_FILE or assertion upon adding partition to period table
+--echo #
+create table t1 (f date, t date, period for app(f,t)) with system versioning partition by system_time ( partition p1 history, partition pn current );
+lock table t1 write;
+alter table t1 add partition (partition p2 history);
+unlock tables;
+
+create or replace table t1 (x int, s date, e date, period for app(s,e));
+insert into t1 values(1, '2020-03-01', '2020-03-02');
+insert into t1 values(1, '2020-03-01', '2020-03-02');
+--error ER_DUP_ENTRY
+alter table t1 add primary key(x, s, e);
+alter table t1 add system versioning;
+
+# cleanup
+drop table t1;

sql/field.cc

@@ -11387,3 +11387,20 @@ void Field::print_key_value_binary(String *out, const uchar* key, uint32 length)
 {
   out->append_semi_hex((const char*)key, length, charset());
 }
+
+
+Virtual_column_info* Virtual_column_info::clone(THD *thd)
+{
+  Virtual_column_info* dst= new (thd->mem_root) Virtual_column_info(*this);
+  if (!dst)
+    return NULL;
+  if (expr)
+  {
+    dst->expr= expr->get_copy(thd);
+    if (!dst->expr)
+      return NULL;
+  }
+  if (!thd->make_lex_string(&dst->name, name.str, name.length))
+    return NULL;
+  return dst;
+};

sql/field.h

@@ -555,6 +555,7 @@ class Virtual_column_info: public Sql_alloc,
     name.str= NULL;
     name.length= 0;
   };
+  Virtual_column_info* clone(THD *thd);
   ~Virtual_column_info() {};
   enum_vcol_info_type get_vcol_type() const
   {

sql/sql_partition.cc

@@ -6875,6 +6875,7 @@ static void alter_partition_lock_handling(ALTER_PARTITION_PARAM_TYPE *lpt)
       thd->set_stmt_da(&tmp_stmt_da);
     }
 
+    // TODO: why error status of reopen_tables() is ignored?
     if (unlikely(thd->locked_tables_list.reopen_tables(thd, false)))
       sql_print_warning("We failed to reacquire LOCKs in ALTER TABLE");
 

sql/sql_table.cc

@@ -4311,6 +4311,8 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
         const Virtual_column_info *dup_check;
         while ((dup_check= dup_it++) && dup_check != check)
         {
+          if (!dup_check->name.length || dup_check->automatic_name)
+            continue;
           if (!lex_string_cmp(system_charset_info,
                               &check->name, &dup_check->name))
           {
@@ -8480,8 +8482,8 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
 	  key_part_length= 0;			// Use whole field
       }
       key_part_length /= kfield->charset()->mbmaxlen;
-      key_parts.push_back(new Key_part_spec(&cfield->field_name,
-					    key_part_length),
+      key_parts.push_back(new (thd->mem_root) Key_part_spec(
+                            &cfield->field_name, key_part_length),
                           thd->mem_root);
     }
     if (table->s->tmp_table == NO_TMP_TABLE)
@@ -8547,7 +8549,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
       tmp_name.str= key_name;
       tmp_name.length= strlen(key_name);
       /* We dont need LONG_UNIQUE_HASH_FIELD flag because it will be autogenerated */
-      key= new Key(key_type, &tmp_name, &key_create_info,
+      key= new (thd->mem_root) Key(key_type, &tmp_name, &key_create_info,
                    MY_TEST(key_info->flags & HA_GENERATED_KEY),
                    &key_parts, key_info->option_list, DDL_options());
       new_key_list.push_back(key, thd->mem_root);
@@ -8627,26 +8629,37 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
         }
       }
 
+      // NB: `check` is TABLE resident, we must keep it intact.
+      if (keep)
+      {
+        check= check->clone(thd);
+        if (!check)
+        {
+          my_error(ER_OUT_OF_RESOURCES, MYF(0));
+          goto err;
+        }
+      }
+
       if (share->period.constr_name.streq(check->name.str))
       {
-        if (!drop_period && !keep)
+        if (drop_period)
+        {
+          keep= false;
+        }
+        else if(!keep)
         {
           my_error(ER_PERIOD_CONSTRAINT_DROP, MYF(0), check->name.str,
                    share->period.name.str);
           goto err;
         }
-        keep= keep && !drop_period;
-
-        DBUG_ASSERT(create_info->period_info.constr == NULL || drop_period);
-
-        if (keep)
+        else
         {
-          Item *expr_copy= check->expr->get_copy(thd);
-          check= new Virtual_column_info();
-          check->expr= expr_copy;
+          DBUG_ASSERT(create_info->period_info.constr == NULL);
           create_info->period_info.constr= check;
+          create_info->period_info.constr->automatic_name= true;
         }
       }
+
       /* see if the constraint depends on *only* on dropped fields */
       if (keep && dropped_fields)
       {

sql/unireg.cc

@@ -57,6 +57,13 @@ static bool make_empty_rec(THD *, uchar *, uint, List<Create_field> &, uint,
 */
 static uchar *extra2_write_len(uchar *pos, size_t len)
 {
+  /* TODO: should be
+     if (len > 0 && len <= 255)
+       *pos++= (uchar)len;
+     ...
+     because extra2_read_len() uses 0 for 2-byte lengths.
+     extra2_str_size() must be fixed too.
+  */
   if (len <= 255)
     *pos++= (uchar)len;
   else