Skip to content

Commit

Permalink
ddl: support modify column on partitioned table (#38302)
Browse files Browse the repository at this point in the history
close #38297
  • Loading branch information
mjonss authored Oct 13, 2022
1 parent f45f497 commit af714c2
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 13 deletions.
3 changes: 3 additions & 0 deletions ddl/backfilling.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,9 @@ func (dc *ddlCtx) handleRangeTasks(sessPool *sessionPool, t table.Table, workers
physicalTableID := reorgInfo.PhysicalTableID

var prefix kv.Key
if tbl, ok := t.(table.PartitionedTable); ok {
t = tbl.GetPartition(physicalTableID)
}
if reorgInfo.mergingTmpIdx {
prefix = t.IndexPrefix()
} else {
Expand Down
38 changes: 31 additions & 7 deletions ddl/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -1020,9 +1020,30 @@ func BuildElements(changingCol *model.ColumnInfo, changingIdxs []*model.IndexInf
return elements
}

func (w *worker) updatePhysicalTableRow(t table.PhysicalTable, reorgInfo *reorgInfo) error {
func (w *worker) updatePhysicalTableRow(t table.Table, reorgInfo *reorgInfo) error {
logutil.BgLogger().Info("[ddl] start to update table row", zap.String("job", reorgInfo.Job.String()), zap.String("reorgInfo", reorgInfo.String()))
return w.writePhysicalTableRecord(w.sessPool, t, typeUpdateColumnWorker, reorgInfo)
if tbl, ok := t.(table.PartitionedTable); ok {
done := false
for !done {
p := tbl.GetPartition(reorgInfo.PhysicalTableID)
if p == nil {
return dbterror.ErrCancelledDDLJob.GenWithStack("Can not find partition id %d for table %d", reorgInfo.PhysicalTableID, t.Meta().ID)
}
err := w.writePhysicalTableRecord(w.sessPool, p, typeUpdateColumnWorker, reorgInfo)
if err != nil {
return err
}
done, err = w.updateReorgInfo(tbl, reorgInfo)
if err != nil {
return errors.Trace(err)
}
}
return nil
}
if tbl, ok := t.(table.PhysicalTable); ok {
return w.writePhysicalTableRecord(w.sessPool, tbl, typeUpdateColumnWorker, reorgInfo)
}
return dbterror.ErrCancelledDDLJob.GenWithStack("internal error for phys tbl id: %d tbl id: %d", reorgInfo.PhysicalTableID, t.Meta().ID)
}

// TestReorgGoroutineRunning is only used in test to indicate the reorg goroutine has been started.
Expand All @@ -1044,22 +1065,25 @@ func (w *worker) updateCurrentElement(t table.Table, reorgInfo *reorgInfo) error
}
}
})
// TODO: Support partition tables.
if bytes.Equal(reorgInfo.currElement.TypeKey, meta.ColumnElementKey) {
//nolint:forcetypeassert
err := w.updatePhysicalTableRow(t.(table.PhysicalTable), reorgInfo)
err := w.updatePhysicalTableRow(t, reorgInfo)
if err != nil {
return errors.Trace(err)
}
}

var physTbl table.PhysicalTable
if tbl, ok := t.(table.PartitionedTable); ok {
physTbl = tbl.GetPartition(reorgInfo.PhysicalTableID)
} else if tbl, ok := t.(table.PhysicalTable); ok {
physTbl = tbl
}
// Get the original start handle and end handle.
currentVer, err := getValidCurrentVersion(reorgInfo.d.store)
if err != nil {
return errors.Trace(err)
}
//nolint:forcetypeassert
originalStartHandle, originalEndHandle, err := getTableRange(reorgInfo.d.jobContext(reorgInfo.Job), reorgInfo.d, t.(table.PhysicalTable), currentVer.Ver, reorgInfo.Job.Priority)
originalStartHandle, originalEndHandle, err := getTableRange(reorgInfo.d.jobContext(reorgInfo.Job), reorgInfo.d, physTbl, currentVer.Ver, reorgInfo.Job.Priority)
if err != nil {
return errors.Trace(err)
}
Expand Down
128 changes: 128 additions & 0 deletions ddl/db_partition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4528,3 +4528,131 @@ func TestPartitionTableWithAnsiQuotes(t *testing.T) {
` PARTITION "p4" VALUES LESS THAN ('\\''\t\n','\\''\t\n'),` + "\n" +
` PARTITION "pMax" VALUES LESS THAN (MAXVALUE,MAXVALUE))`))
}
func TestAlterModifyColumnOnPartitionedTable(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("create database AlterPartTable")
tk.MustExec("use AlterPartTable")
tk.MustExec(`create table t (a int unsigned PRIMARY KEY, b varchar(255), key (b))`)
tk.MustExec(`insert into t values (7, "07"), (8, "08"),(23,"23"),(34,"34💥"),(46,"46"),(57,"57")`)
tk.MustQuery(`show create table t`).Check(testkit.Rows(
"t CREATE TABLE `t` (\n" +
" `a` int(10) unsigned NOT NULL,\n" +
" `b` varchar(255) DEFAULT NULL,\n" +
" PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" +
" KEY `b` (`b`)\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"))
// TODO: Why does it allow 💥 as a latin1 character?
tk.MustQuery(`select hex(b) from t where a = 34`).Check(testkit.Rows("3334F09F92A5"))
tk.MustExec(`alter table t modify b varchar(200) charset latin1`)
tk.MustQuery(`show create table t`).Check(testkit.Rows(
"t CREATE TABLE `t` (\n" +
" `a` int(10) unsigned NOT NULL,\n" +
" `b` varchar(200) CHARACTER SET latin1 COLLATE latin1_bin DEFAULT NULL,\n" +
" PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" +
" KEY `b` (`b`)\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin"))
tk.MustQuery(`select hex(b) from t where a = 34`).Check(testkit.Rows("3334F09F92A5"))
tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+
"23 23",
"34 34💥",
"46 46",
"57 57",
"7 07",
"8 08"))
tk.MustQuery(`select * from t order by b`).Check(testkit.Rows(""+
"7 07",
"8 08",
"23 23",
"34 34💥",
"46 46",
"57 57"))
tk.MustExec(`alter table t change b c varchar(200) charset utf8mb4`)
tk.MustExec(`drop table t`)
tk.MustExec(`create table t (a int unsigned PRIMARY KEY, b varchar(255), key (b)) partition by range (a) ` +
`(partition p0 values less than (10),` +
` partition p1 values less than (20),` +
` partition p2 values less than (30),` +
` partition pMax values less than (MAXVALUE))`)
tk.MustExec(`insert into t values (7, "07"), (8, "08"),(23,"23"),(34,"34💥"),(46,"46"),(57,"57")`)
tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+
"23 23",
"34 34💥",
"46 46",
"57 57",
"7 07",
"8 08"))
tk.MustQuery(`select * from t order by b`).Check(testkit.Rows(""+
"7 07",
"8 08",
"23 23",
"34 34💥",
"46 46",
"57 57"))
tk.MustQuery(`show create table t`).Check(testkit.Rows(
"t CREATE TABLE `t` (\n" +
" `a` int(10) unsigned NOT NULL,\n" +
" `b` varchar(255) DEFAULT NULL,\n" +
" PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" +
" KEY `b` (`b`)\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
"PARTITION BY RANGE (`a`)\n" +
"(PARTITION `p0` VALUES LESS THAN (10),\n" +
" PARTITION `p1` VALUES LESS THAN (20),\n" +
" PARTITION `p2` VALUES LESS THAN (30),\n" +
" PARTITION `pMax` VALUES LESS THAN (MAXVALUE))"))
tk.MustExec(`alter table t modify b varchar(200) charset latin1`)
tk.MustQuery(`show create table t`).Check(testkit.Rows(
"t CREATE TABLE `t` (\n" +
" `a` int(10) unsigned NOT NULL,\n" +
" `b` varchar(200) CHARACTER SET latin1 COLLATE latin1_bin DEFAULT NULL,\n" +
" PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" +
" KEY `b` (`b`)\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
"PARTITION BY RANGE (`a`)\n" +
"(PARTITION `p0` VALUES LESS THAN (10),\n" +
" PARTITION `p1` VALUES LESS THAN (20),\n" +
" PARTITION `p2` VALUES LESS THAN (30),\n" +
" PARTITION `pMax` VALUES LESS THAN (MAXVALUE))"))
tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+
"23 23",
"34 34💥",
"46 46",
"57 57",
"7 07",
"8 08"))
tk.MustQuery(`select * from t order by b`).Check(testkit.Rows(""+
"7 07",
"8 08",
"23 23",
"34 34💥",
"46 46",
"57 57"))
tk.MustExec(`alter table t change b c varchar(150) charset utf8mb4`)
tk.MustQuery(`show create table t`).Check(testkit.Rows(
"t CREATE TABLE `t` (\n" +
" `a` int(10) unsigned NOT NULL,\n" +
" `c` varchar(150) DEFAULT NULL,\n" +
" PRIMARY KEY (`a`) /*T![clustered_index] CLUSTERED */,\n" +
" KEY `b` (`c`)\n" +
") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin\n" +
"PARTITION BY RANGE (`a`)\n" +
"(PARTITION `p0` VALUES LESS THAN (10),\n" +
" PARTITION `p1` VALUES LESS THAN (20),\n" +
" PARTITION `p2` VALUES LESS THAN (30),\n" +
" PARTITION `pMax` VALUES LESS THAN (MAXVALUE))"))
tk.MustQuery(`select * from t`).Sort().Check(testkit.Rows(""+
"23 23",
"34 34💥",
"46 46",
"57 57",
"7 07",
"8 08"))
tk.MustQuery(`select * from t order by c`).Check(testkit.Rows(""+
"7 07",
"8 08",
"23 23",
"34 34💥",
"46 46",
"57 57"))
}
3 changes: 0 additions & 3 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4543,9 +4543,6 @@ func GetModifiableColumnJob(
if err = isGeneratedRelatedColumn(t.Meta(), newCol.ColumnInfo, col.ColumnInfo); err != nil {
return nil, errors.Trace(err)
}
if t.Meta().Partition != nil {
return nil, dbterror.ErrUnsupportedModifyColumn.GenWithStackByArgs("table is partition table")
}
}

// We don't support modifying column from not_auto_increment to auto_increment.
Expand Down
4 changes: 1 addition & 3 deletions ddl/failtest/fail_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,6 @@ func TestModifyColumn(t *testing.T) {
tk.MustExec("admin check table t")

// Test unsupported statements.
tk.MustExec("create table t1(a int) partition by hash (a) partitions 2")
tk.MustGetErrMsg("alter table t1 modify column a mediumint", "[ddl:8200]Unsupported modify column: table is partition table")
tk.MustExec("create table t2(id int, a int, b int generated always as (abs(a)) virtual, c int generated always as (a+1) stored)")
tk.MustGetErrMsg("alter table t2 modify column b mediumint", "[ddl:8200]Unsupported modify column: newCol IsGenerated false, oldCol IsGenerated true")
tk.MustGetErrMsg("alter table t2 modify column c mediumint", "[ddl:8200]Unsupported modify column: newCol IsGenerated false, oldCol IsGenerated true")
Expand Down Expand Up @@ -531,7 +529,7 @@ func TestModifyColumn(t *testing.T) {
tk.MustExec("insert into t5 values (1,1),(2,2),(3,3),(4,4),(5,5);")
tk.MustExec("alter table t5 modify a int not null;")

tk.MustExec("drop table t, t1, t2, t3, t4, t5")
tk.MustExec("drop table t, t2, t3, t4, t5")
}

func TestPartitionAddPanic(t *testing.T) {
Expand Down

0 comments on commit af714c2

Please sign in to comment.