Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-6103

Adding/removing non-materialized virtual column triggers table recreation

Details

    • Bug
    • Status: Closed (View Workflow)
    • Critical
    • Resolution: Fixed
    • 5.5.36, 10.0.10
    • 10.0.12
    • None
    • None
    • 32-Bit Debian 7

    Description

      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.

      Attachments

        Issue Links

          Activity

            hsc Horst Schirmeier created issue -
            serg Sergei Golubchik made changes -
            Field Original Value New Value
            Description 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.
            serg Sergei Golubchik made changes -
            Fix Version/s 10.0.11 [ 15200 ]
            Fix Version/s 5.5.38 [ 15400 ]
            serg Sergei Golubchik made changes -
            Priority Minor [ 4 ] Major [ 3 ]
            serg Sergei Golubchik made changes -
            Fix Version/s 10.0.12 [ 15201 ]
            Fix Version/s 10.0.11 [ 15200 ]
            serg Sergei Golubchik made changes -
            Assignee Sergey Vojtovich [ svoj ]

            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 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 Sergey Vojtovich made changes -
            Attachment mdev6103.patch [ 30502 ]
            svoj Sergey Vojtovich made changes -
            Assignee Sergey Vojtovich [ svoj ] Sergei Golubchik [ serg ]

            Sergei, please voice your suggestions. I see no simple solution for this bug.

            svoj Sergey Vojtovich added a comment - Sergei, please voice your suggestions. I see no simple solution for this bug.

            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 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 Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Sergey Vojtovich [ svoj ]

            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.

            svoj 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;

            serg 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.

            svoj 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?

            svoj 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.

            svoj 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.
            svoj Sergey Vojtovich made changes -
            Fix Version/s 5.5.38 [ 15400 ]
            svoj Sergey Vojtovich made changes -
            Assignee Sergey Vojtovich [ svoj ] Sergei Golubchik [ serg ]

            serg, please review fix for this bug.

            svoj Sergey Vojtovich added a comment - serg , please review fix for this bug.
            serg Sergei Golubchik made changes -
            Priority Major [ 3 ] Critical [ 2 ]

            ok to push!

            serg Sergei Golubchik added a comment - ok to push!
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Sergey Vojtovich [ svoj ]

            Fixed in 10.0.12:

            revno: 4218
            revision-id: svoj@mariadb.org-20140603125729-k5htqiobnba0wqsj
            parent: psergey@askmonty.org-20140603202627-ior1c69y85kz1ggd
            committer: Sergey Vojtovich <svoj@mariadb.org>
            branch nick: 10.0-mdev6103
            timestamp: Tue 2014-06-03 16:57:29 +0400
            message:
              MDEV-6103 - Adding/removing non-materialized virtual column triggers
                          table recreation
             
              Relaxed InnoDB/XtraDB checks to allow online add/drop of
              non-materialized virtual columns.

            svoj Sergey Vojtovich added a comment - Fixed in 10.0.12: revno: 4218 revision-id: svoj@mariadb.org-20140603125729-k5htqiobnba0wqsj parent: psergey@askmonty.org-20140603202627-ior1c69y85kz1ggd committer: Sergey Vojtovich <svoj@mariadb.org> branch nick: 10.0-mdev6103 timestamp: Tue 2014-06-03 16:57:29 +0400 message: MDEV-6103 - Adding/removing non-materialized virtual column triggers table recreation   Relaxed InnoDB/XtraDB checks to allow online add/drop of non-materialized virtual columns.
            svoj Sergey Vojtovich made changes -
            Resolution Fixed [ 1 ]
            Status Open [ 1 ] Closed [ 6 ]
            svoj Sergey Vojtovich made changes -

            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.

            hsc 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.
            svoj Sergey Vojtovich added a comment - See MDEV-6303 .
            serg Sergei Golubchik made changes -
            Workflow defaullt [ 38918 ] MariaDB v2 [ 43917 ]
            ratzpo Rasmus Johansson (Inactive) made changes -
            Workflow MariaDB v2 [ 43917 ] MariaDB v3 [ 63151 ]
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 63151 ] MariaDB v4 [ 147796 ]

            People

              svoj Sergey Vojtovich
              hsc Horst Schirmeier
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.