When adding or removing a non-materialized virtual column to/from a MyISAM table with ALTER TABLE, MariaDB recreates the whole table although only the metadata should be affected:
MariaDB [test]> ALTER TABLE test ADD COLUMN testcolumn INT AS (othercolumn+1) VIRTUAL;
Stage: 1 of 2 'copy to tmp table' 0.001% of stage done
This is especially annoying for multi-gigabyte tables, where this operation takes a long time.
When adding or removing a non-materialized virtual column to/from a MyISAM table with ALTER TABLE, MariaDB recreates the whole table although only the metadata should be affected:
MariaDB [test]> ALTER TABLE test ADD COLUMN testcolumn INT AS (othercolumn+1) VIRTUAL;
Stage: 1 of 2 'copy to tmp table' 0.001% of stage done
This is especially annoying for multi-gigabyte tables, where this operation takes a long time.
When adding or removing a non-materialized virtual column to/from a MyISAM table with ALTER TABLE, MariaDB recreates the whole table although only the metadata should be affected:
{noformat}
MariaDB [test]> ALTER TABLE test ADD COLUMN testcolumn INT AS (othercolumn+1) VIRTUAL;
Stage: 1 of 2 'copy to tmp table' 0.001% of stage done
{noformat}
This is especially annoying for multi-gigabyte tables, where this operation takes a long time.
It came to be that MyISAM (and most probably Aria) reserves extra bits for virtual columns (NULL flag) in a physical record (the one that is written to disk). There should be no good reason to do so.
E.g.:
create table t1 (b int, d int); # Uses 2nd and 3rd bits for NULL flags
create table t2 (a int as (-b), b int, c int as (-b), d int); # Uses 3rd and 5th bits for NULL flags
All in all it means physical record format changes when adding/dropping virtual columns and just relaxing table comparison function is not enough to fix this bug.
I will attach partial fix for this bug shortly. InnoDB doesn't seem to be affected.
Sergey Vojtovich
added a comment - It came to be that MyISAM (and most probably Aria) reserves extra bits for virtual columns (NULL flag) in a physical record (the one that is written to disk). There should be no good reason to do so.
E.g.:
create table t1 (b int, d int); # Uses 2nd and 3rd bits for NULL flags
create table t2 (a int as (-b), b int, c int as (-b), d int); # Uses 3rd and 5th bits for NULL flags
All in all it means physical record format changes when adding/dropping virtual columns and just relaxing table comparison function is not enough to fix this bug.
I will attach partial fix for this bug shortly. InnoDB doesn't seem to be affected.
svoj As far as I understand, your patch fixes this issue for InnoDB and other storage engines that don't store MariaDB rows natively. While MyISAM and Aria still be affected?
As far as the patch is concerned — it's pretty much ok. I don't see mysql-test/suite/vcol/r/vcol_non_stored_columns_myisam.result in it, and I'd suggest to move stored_in_db || is_in_partitioning_expr() into a separate inline helper function or method, but this is a minor detail.
MyISAM/Aria — let's do it in a separate Jira issue, the fix would more intrusive and more risky, so it could only go into 10.1 not earlier.
Sergei Golubchik
added a comment - svoj As far as I understand, your patch fixes this issue for InnoDB and other storage engines that don't store MariaDB rows natively. While MyISAM and Aria still be affected?
As far as the patch is concerned — it's pretty much ok. I don't see mysql-test/suite/vcol/r/vcol_non_stored_columns_myisam.result in it, and I'd suggest to move stored_in_db || is_in_partitioning_expr() into a separate inline helper function or method, but this is a minor detail.
MyISAM/Aria — let's do it in a separate Jira issue, the fix would more intrusive and more risky, so it could only go into 10.1 not earlier.
serg My patch relaxes comparison function so that ADD/DROP virtual column is considered meta-data change. Such ALTER should work well with InnoDB, but it corrupts meta-data for MyISAM/Aria. In fact vcol_non_stored_columns_myisam fails with my patch.
We could keep old behavior for MyISAM/Aria, but FWICS we'll have to hardcode it.
Sergey Vojtovich
added a comment - serg My patch relaxes comparison function so that ADD/DROP virtual column is considered meta-data change. Such ALTER should work well with InnoDB, but it corrupts meta-data for MyISAM/Aria. In fact vcol_non_stored_columns_myisam fails with my patch.
We could keep old behavior for MyISAM/Aria, but FWICS we'll have to hardcode it.
You could add to ha_myisam::check_if_incompatible_data() the missing check — the one, that you removed from sql_table.cc:
if (table->s->fields != alter_info->create_list.elements)
return COMPATIBLE_DATA_NO;
Sergei Golubchik
added a comment - You could add to ha_myisam::check_if_incompatible_data() the missing check — the one, that you removed from sql_table.cc:
if (table->s->fields != alter_info->create_list.elements)
return COMPATIBLE_DATA_NO;
serg Indeed, I was confused by fact that Alter_info is not available there. But create_info should be just fine perform this check.
Sergey Vojtovich
added a comment - serg Indeed, I was confused by fact that Alter_info is not available there. But create_info should be just fine perform this check.
serg Hmm... it came to be that HA_CREATE_INFO doesn't store number of fields in altered definition. Should we extend HA_CREATE_INFO?
Sergey Vojtovich
added a comment - serg Hmm... it came to be that HA_CREATE_INFO doesn't store number of fields in altered definition. Should we extend HA_CREATE_INFO?
Removing 5.5 from "Fix versions": storage engines API of 5.5 is not flexible enough to solve this problem.
Adding virtual column to InnoDB table in 10.0 doesn't perform table copy. Whereas dropping virtual column does.
It should be sufficient to fix ha_innobase::check_if_supported_inplace_alter.
Sergey Vojtovich
added a comment - Removing 5.5 from "Fix versions": storage engines API of 5.5 is not flexible enough to solve this problem.
Adding virtual column to InnoDB table in 10.0 doesn't perform table copy. Whereas dropping virtual column does.
It should be sufficient to fix ha_innobase::check_if_supported_inplace_alter.
Shouldn't this bug be split? As I understand, this issue is now fixed for InnoDB. I saw this issue with MyISAM, which is still affected.
Horst Schirmeier
added a comment - Shouldn't this bug be split? As I understand, this issue is now fixed for InnoDB. I saw this issue with MyISAM, which is still affected.
Bug
Critical
It came to be that MyISAM (and most probably Aria) reserves extra bits for virtual columns (NULL flag) in a physical record (the one that is written to disk). There should be no good reason to do so.
E.g.:
create table t1 (b int, d int); # Uses 2nd and 3rd bits for NULL flags
create table t2 (a int as (-b), b int, c int as (-b), d int); # Uses 3rd and 5th bits for NULL flags
All in all it means physical record format changes when adding/dropping virtual columns and just relaxing table comparison function is not enough to fix this bug.
I will attach partial fix for this bug shortly. InnoDB doesn't seem to be affected.