From fa0cada95baeb9fad5a2d6dad687bb13c93da9b6 Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Fri, 7 Oct 2022 00:45:21 +0300 Subject: [PATCH] MDEV-28576 RENAME COLUMN with NOCOPY algorithm leads to corrupt partitioned table 10.5 part: test cases and comments. The code is in the merge commit 74fe1c44aa1 When f.ex. table is partitioned by HASH(a) and we rename column `a' to `b' partitioning filter stays unchanged: HASH(a). That's the wrong behavior. The patch updates partitioning filter in accordance to the new columns names. That includes partition/subpartition expression and partition/subpartition field list. --- mysql-test/main/partition_alter.result | 78 ++++++++++++++++++++++++++ mysql-test/main/partition_alter.test | 41 ++++++++++++++ sql/field.h | 2 +- sql/partition_info.h | 8 +++ sql/sql_partition.cc | 1 + sql/sql_table.cc | 6 ++ 6 files changed, 135 insertions(+), 1 deletion(-) diff --git a/mysql-test/main/partition_alter.result b/mysql-test/main/partition_alter.result index 2b0a09d2653..539f896f54f 100644 --- a/mysql-test/main/partition_alter.result +++ b/mysql-test/main/partition_alter.result @@ -212,3 +212,81 @@ test.t check status OK delete from t order by b limit 1; drop table t; # End of 10.3 tests +# +# MDEV-28576 RENAME COLUMN with NOCOPY algorithm leads to corrupt partitioned table +# +create table t (a int) +partition by list (a) +subpartition by hash(a) subpartitions 2 +(partition p0 values in (1)); +alter table t rename column a to b, algorithm=nocopy; +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `b` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci + PARTITION BY LIST (`b`) +SUBPARTITION BY HASH (`b`) +SUBPARTITIONS 2 +(PARTITION `p0` VALUES IN (1) ENGINE = MyISAM) +alter table t rename column b to c, algorithm=copy; +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `c` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci + PARTITION BY LIST (`c`) +SUBPARTITION BY HASH (`c`) +SUBPARTITIONS 2 +(PARTITION `p0` VALUES IN (1) ENGINE = MyISAM) +drop table t; +create table t (d int, e int) +partition by list columns (d, e) +subpartition by key (d, e) +(partition p0 values in ((2, 3))); +alter table t rename column d to f, rename column e to g, algorithm=nocopy; +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `f` int(11) DEFAULT NULL, + `g` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci + PARTITION BY LIST COLUMNS(`f`,`g`) +SUBPARTITION BY KEY (`f`,`g`) +(PARTITION `p0` VALUES IN ((2,3)) ENGINE = MyISAM) +alter table t rename column f to h, rename column g to i, algorithm=copy; +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `h` int(11) DEFAULT NULL, + `i` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci + PARTITION BY LIST COLUMNS(`h`,`i`) +SUBPARTITION BY KEY (`h`,`i`) +(PARTITION `p0` VALUES IN ((2,3)) ENGINE = MyISAM) +drop table t; +create table t (k int, l int) +partition by range (k) +subpartition by hash(l) subpartitions 4 +(partition p0 values less than (5)); +alter table t rename column k to l, rename column l to k; +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `l` int(11) DEFAULT NULL, + `k` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci + PARTITION BY RANGE (`l`) +SUBPARTITION BY HASH (`k`) +SUBPARTITIONS 4 +(PARTITION `p0` VALUES LESS THAN (5) ENGINE = MyISAM) +drop table t; +create table t (a int, b int) partition by list (b) (partition p1 values in (1, 2)); +insert into t values (0, 1), (2, 2); +alter table t rename column b to f, rename column a to b, algorithm=nocopy; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +delete from t order by b limit 1; +drop table t; +# End of 10.5 tests diff --git a/mysql-test/main/partition_alter.test b/mysql-test/main/partition_alter.test index ded6fde4794..4f17b520489 100644 --- a/mysql-test/main/partition_alter.test +++ b/mysql-test/main/partition_alter.test @@ -204,3 +204,44 @@ delete from t order by b limit 1; drop table t; --echo # End of 10.3 tests + +--echo # +--echo # MDEV-28576 RENAME COLUMN with NOCOPY algorithm leads to corrupt partitioned table +--echo # +create table t (a int) +partition by list (a) +subpartition by hash(a) subpartitions 2 +(partition p0 values in (1)); +alter table t rename column a to b, algorithm=nocopy; +show create table t; +alter table t rename column b to c, algorithm=copy; +show create table t; +drop table t; + +create table t (d int, e int) +partition by list columns (d, e) +subpartition by key (d, e) +(partition p0 values in ((2, 3))); +alter table t rename column d to f, rename column e to g, algorithm=nocopy; +show create table t; +alter table t rename column f to h, rename column g to i, algorithm=copy; +show create table t; +drop table t; + +create table t (k int, l int) +partition by range (k) +subpartition by hash(l) subpartitions 4 +(partition p0 values less than (5)); +alter table t rename column k to l, rename column l to k; +show create table t; +drop table t; + +create table t (a int, b int) partition by list (b) (partition p1 values in (1, 2)); +insert into t values (0, 1), (2, 2); +alter table t rename column b to f, rename column a to b, algorithm=nocopy; +check table t; +delete from t order by b limit 1; +# cleanup +drop table t; + +--echo # End of 10.5 tests diff --git a/sql/field.h b/sql/field.h index 8b7c6d6a554..e1b67d6acc1 100644 --- a/sql/field.h +++ b/sql/field.h @@ -5661,7 +5661,7 @@ inline bool Row_definition_list::eq_name(const Spvar_definition *def, class Create_field :public Column_definition { public: - LEX_CSTRING change; // If done with alter table + LEX_CSTRING change; // Old column name if column is renamed by ALTER LEX_CSTRING after; // Put column after this one Field *field; // For alter table const TYPELIB *save_interval; // Temporary copy for the above diff --git a/sql/partition_info.h b/sql/partition_info.h index 55ea20cf681..aebb81ff9c5 100644 --- a/sql/partition_info.h +++ b/sql/partition_info.h @@ -79,6 +79,10 @@ struct Vers_part_info : public Sql_alloc partition_element *hist_part; }; +/* + See generate_partition_syntax() for details of how the data is used + in partition expression. +*/ class partition_info : public Sql_alloc { public: @@ -88,6 +92,10 @@ public: List partitions; List temp_partitions; + /* + These are mutually exclusive with part_expr/subpart_expr depending on + what is specified in partitioning filter: expression or column list. + */ List part_field_list; List subpart_field_list; diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 78f56992f46..f004981bd2a 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -4801,6 +4801,7 @@ static void check_datadir_altered_for_innodb(THD *thd, @param[out] partition_changed Boolean indicating whether partition changed @param[out] fast_alter_table Boolean indicating if fast partition alter is possible. + @param[out] thd->work_part_info Prepared part_info for the new table @return Operation status @retval TRUE Error diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 5759e21c0fc..b5edd5bba6c 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -10535,6 +10535,9 @@ do_continue:; #ifdef WITH_PARTITION_STORAGE_ENGINE { + /* + Partitioning: part_info is prepared and returned via thd->work_part_info + */ if (prep_alter_part_table(thd, table, alter_info, create_info, &partition_changed, &fast_alter_partition)) { @@ -10712,6 +10715,9 @@ do_continue:; else alter_info->flags|= ALTER_INDEX_ORDER; create_info->alias= alter_ctx.table_name; + /* + Partitioning: part_info is passed via thd->work_part_info + */ error= create_table_impl(thd, alter_ctx.db, alter_ctx.table_name, alter_ctx.new_db, alter_ctx.tmp_name, alter_ctx.get_tmp_path(),