MDEV-34632 Assertion failed in handler::assert_icp_limitations

Assertion `table->field[0]->ptr >= table->record[0] &&
table->field[0]->ptr <= table->record[0] + table->s->reclength' failed in
handler::assert_icp_limitations.

table->move_fields has some limitations:
1. It cannot be used in cascade
2. It should always have a restoring pair.

Rule 1 is covered by assertions in handler::assert_icp_limitations
and handler::ptr_in_record (commit 30894fe9).

Rule 2 should be manually maintained with care. Hopefully, the rule 1 assertions
may sometimes help as well.

In ha_myisam::repair, both rules are broken. table->move_fields is used
asymmetrically there: it is set on every param->fix_record call
(i.e. in compute_vcols) but is restored only once, in the end of repair.

The reason to updating field ptr's for every call is that compute_vcols can
(supposedly) be called in parallel, that is, with the same table, but different
records.

The condition to "unmove" the pointers in ha_myisam::restore_vcos_after_repair
is incorrect, when stored vcols are available, and myisam stores a VIRTUAL field
if it's the only field in the table (the record cannot be of zero length).

This patch solves the problem by "unmoving" the pointers symmetrically, in
compute_vcols. That is, both rules will be preserved maintained.

mysql-test/suite/vcol/inc/vcol_keys.inc

@@ -15,6 +15,8 @@
 ################################################################################
 
 
+--source include/have_sequence.inc
+
 --echo #            - UNIQUE KEY
 --echo #            - INDEX
 --echo #            - FULLTEXT INDEX
@@ -231,3 +233,18 @@ CREATE TABLE t1 (
 DELETE FROM t1 WHERE vc IS NULL ORDER BY pk;
 DROP TABLE t1;
 
+#
+# MDEV-34632 Assertion `table->field[0]->ptr >= table->record[0] &&
+# table->field[0]->ptr <= table->record[0] + table->s->reclength' failed in
+# void handler::assert_icp_limitations(uchar*)
+#
+SET sql_mode='';
+CREATE TABLE t1 (a INT GENERATED ALWAYS AS (1) VIRTUAL,KEY(a)) ENGINE=MyISAM;
+INSERT INTO t1 SELECT 1 FROM seq_1_to_2 ;
+SELECT * FROM t1;
+DROP TABLE t1;
+
+CREATE TABLE t1 (a INT GENERATED ALWAYS AS (1) VIRTUAL,KEY(a)) ENGINE=MyISAM;
+INSERT INTO t1 SELECT 1 UNION SELECT 1;
+SELECT * FROM t1;
+DROP TABLE t1;

mysql-test/suite/vcol/r/vcol_keys_innodb.result

@@ -215,6 +215,25 @@ INDEX(vc(32))
 );
 DELETE FROM t1 WHERE vc IS NULL ORDER BY pk;
 DROP TABLE t1;
+SET sql_mode='';
+CREATE TABLE t1 (a INT GENERATED ALWAYS AS (1) VIRTUAL,KEY(a)) ENGINE=MyISAM;
+INSERT INTO t1 SELECT 1 FROM seq_1_to_2 ;
+Warnings:
+Warning	1906	The value specified for generated column 'a' in table 't1' has been ignored
+Warning	1906	The value specified for generated column 'a' in table 't1' has been ignored
+SELECT * FROM t1;
+a
+1
+1
+DROP TABLE t1;
+CREATE TABLE t1 (a INT GENERATED ALWAYS AS (1) VIRTUAL,KEY(a)) ENGINE=MyISAM;
+INSERT INTO t1 SELECT 1 UNION SELECT 1;
+Warnings:
+Warning	1906	The value specified for generated column 'a' in table 't1' has been ignored
+SELECT * FROM t1;
+a
+1
+DROP TABLE t1;
 #
 # MDEV-11737 Failing assertion: block->magic_n == MEM_BLOCK_MAGIC_N
 # 

mysql-test/suite/vcol/r/vcol_keys_myisam.result

@@ -209,6 +209,25 @@ INDEX(vc(32))
 );
 DELETE FROM t1 WHERE vc IS NULL ORDER BY pk;
 DROP TABLE t1;
+SET sql_mode='';
+CREATE TABLE t1 (a INT GENERATED ALWAYS AS (1) VIRTUAL,KEY(a)) ENGINE=MyISAM;
+INSERT INTO t1 SELECT 1 FROM seq_1_to_2 ;
+Warnings:
+Warning	1906	The value specified for generated column 'a' in table 't1' has been ignored
+Warning	1906	The value specified for generated column 'a' in table 't1' has been ignored
+SELECT * FROM t1;
+a
+1
+1
+DROP TABLE t1;
+CREATE TABLE t1 (a INT GENERATED ALWAYS AS (1) VIRTUAL,KEY(a)) ENGINE=MyISAM;
+INSERT INTO t1 SELECT 1 UNION SELECT 1;
+Warnings:
+Warning	1906	The value specified for generated column 'a' in table 't1' has been ignored
+SELECT * FROM t1;
+a
+1
+DROP TABLE t1;
 #
 # Original test
 #

storage/myisam/ha_myisam.cc

@@ -721,29 +721,32 @@ static void init_compute_vcols(void *table)
 
 static int compute_vcols(MI_INFO *info, uchar *record, int keynum)
 {
+  int error= 0;
   /* This mutex is needed for parallel repair */
   mysql_mutex_lock(&info->s->intern_lock);
   TABLE *table= (TABLE*)(info->external_ref);
-  table->move_fields(table->field, record, table->field[0]->record_ptr());
+  table->move_fields(table->field, record, table->record[0]);
   if (keynum == -1) // update all vcols
   {
-    int error= table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_READ);
+    error= table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_READ);
     if (table->update_virtual_fields(table->file, VCOL_UPDATE_INDEXED))
       error= 1;
-    mysql_mutex_unlock(&info->s->intern_lock);
-    return error;
   }
-  // update only one key
-  KEY *key= table->key_info + keynum;
-  KEY_PART_INFO *kp= key->key_part, *end= kp + key->ext_key_parts;
-  for (; kp < end; kp++)
+  else
   {
-    Field *f= table->field[kp->fieldnr - 1];
-    if (f->vcol_info && !f->vcol_info->stored_in_db)
-      table->update_virtual_field(f, false);
+    // update only one key
+    KEY *key= table->key_info + keynum;
+    KEY_PART_INFO *kp= key->key_part, *end= kp + key->ext_key_parts;
+    for (; kp < end; kp++)
+    {
+      Field *f= table->field[kp->fieldnr - 1];
+      if (f->vcol_info && !f->vcol_info->stored_in_db)
+        table->update_virtual_field(f, false);
+    }
   }
+  table->move_fields(table->field, table->record[0], record);
   mysql_mutex_unlock(&info->s->intern_lock);
-  return 0;
+  return error;
 }
 
 }
@@ -1025,16 +1028,6 @@ void ha_myisam::setup_vcols_for_repair(HA_CHECK *param)
   table->use_all_columns();
 }
 
-void ha_myisam::restore_vcos_after_repair()
-{
-  if (file->s->base.reclength < file->s->vreclength)
-  {
-    table->move_fields(table->field, table->record[0],
-                       table->field[0]->record_ptr());
-    table->default_column_bitmaps();
-  }
-}
-
 int ha_myisam::check(THD* thd, HA_CHECK_OPT* check_opt)
 {
   if (!file) return HA_ADMIN_INTERNAL_ERROR;
@@ -1131,8 +1124,6 @@ int ha_myisam::check(THD* thd, HA_CHECK_OPT* check_opt)
     file->update |= HA_STATE_CHANGED | HA_STATE_ROW_CHANGED;
   }
 
-  restore_vcos_after_repair();
-
   thd_proc_info(thd, old_proc_info);
   return error ? HA_ADMIN_CORRUPT : HA_ADMIN_OK;
 }
@@ -1178,8 +1169,6 @@ int ha_myisam::analyze(THD *thd, HA_CHECK_OPT* check_opt)
   else if (!mi_is_crashed(file) && !thd->killed)
     mi_mark_crashed(file);
 
-  restore_vcos_after_repair();
-
   return error ? HA_ADMIN_CORRUPT : HA_ADMIN_OK;
 }
 
@@ -1229,8 +1218,6 @@ int ha_myisam::repair(THD* thd, HA_CHECK_OPT *check_opt)
     break;
   }
 
-  restore_vcos_after_repair();
-
   if (!error && start_records != file->state->records &&
       !(check_opt->flags & T_VERY_SILENT))
   {
@@ -1268,8 +1255,6 @@ int ha_myisam::optimize(THD* thd, HA_CHECK_OPT *check_opt)
     error= repair(thd,*param,1);
   }
 
-  restore_vcos_after_repair();
-
   return error;
 }
 
@@ -1707,8 +1692,6 @@ int ha_myisam::enable_indexes(key_map map, bool persist)
     }
     info(HA_STATUS_CONST);
     thd_proc_info(thd, save_proc_info);
-
-    restore_vcos_after_repair();
   }
   DBUG_RETURN(error);
 }

storage/myisam/ha_myisam.h

@@ -49,7 +49,6 @@ class ha_myisam final : public handler
   bool can_enable_indexes;
   int repair(THD *thd, HA_CHECK &param, bool optimize);
   void setup_vcols_for_repair(HA_CHECK *param);
-  void restore_vcos_after_repair();
 
  public:
   ha_myisam(handlerton *hton, TABLE_SHARE *table_arg);