Skip to content

Commit

Permalink
ddl: cancel add index when can not find partition and fix GetPartitio…
Browse files Browse the repository at this point in the history
…n function bug (#10475) (#10533)
  • Loading branch information
crazycs520 authored and winkyao committed May 20, 2019
1 parent c42547f commit d651e27
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 6 deletions.
18 changes: 17 additions & 1 deletion ddl/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4172,7 +4172,7 @@ func (s *testDBSuite) TestPartitionAddIndex(c *C) {

tk.MustExec("alter table partition_add_idx add index idx1 (hired)")
tk.MustExec("alter table partition_add_idx add index idx2 (id, hired)")
ctx := s.tk.Se.(sessionctx.Context)
ctx := tk.Se.(sessionctx.Context)
is := domain.GetDomain(ctx).InfoSchema()
t, err := is.TableByName(model.NewCIStr("test"), model.NewCIStr("partition_add_idx"))
c.Assert(err, IsNil)
Expand All @@ -4190,6 +4190,22 @@ func (s *testDBSuite) TestPartitionAddIndex(c *C) {

tk.MustExec("admin check table partition_add_idx")
tk.MustExec("drop table partition_add_idx")

// Test hash partition for pr 10475.
tk.MustExec("drop table if exists t1")
defer tk.MustExec("drop table if exists t1")
tk.MustExec("set @@session.tidb_enable_table_partition = '1';")
tk.MustExec("create table t1 (a int,b int, primary key(a)) partition by hash(a) partitions 5;")
tk.MustExec("insert into t1 values (0,0),(1,1),(2,2),(3,3);")
tk.MustExec("alter table t1 add index idx(a)")
tk.MustExec("admin check table t1;")

// Test range partition for pr 10475.
tk.MustExec("drop table t1")
tk.MustExec("create table t1 (a int,b int, primary key(a)) partition by range (a) (partition p0 values less than (10), partition p1 values less than (20));")
tk.MustExec("insert into t1 values (0,0);")
tk.MustExec("alter table t1 add index idx(a)")
tk.MustExec("admin check table t1;")
}

func (s *testDBSuite) TestAlterTableCharset(c *C) {
Expand Down
13 changes: 11 additions & 2 deletions ddl/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,16 @@ func (w *worker) onCreateIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int
return ver, errors.Trace(err)
}

err = w.runReorgJob(t, reorgInfo, d.lease, func() error {
err = w.runReorgJob(t, reorgInfo, d.lease, func() (addIndexErr error) {
defer func() {
r := recover()
if r != nil {
buf := util.GetStack()
logutil.Logger(ddlLogCtx).Error("[ddl] add table index panic", zap.Any("panic", r), zap.String("stack", string(buf)))
metrics.PanicCounter.WithLabelValues(metrics.LabelDDL).Inc()
addIndexErr = errCancelledDDLJob.GenWithStack("add table `%v` index `%v` panic", tblInfo.Name, indexInfo.Name)
}
}()
return w.addTableIndex(tbl, indexInfo, reorgInfo)
})
if err != nil {
Expand Down Expand Up @@ -1177,7 +1186,7 @@ func (w *worker) addTableIndex(t table.Table, idx *model.IndexInfo, reorgInfo *r
for !finish {
p := tbl.GetPartition(reorgInfo.PhysicalTableID)
if p == nil {
return errors.Errorf("Can not find partition id %d for table %d", reorgInfo.PhysicalTableID, t.Meta().ID)
return errCancelledDDLJob.GenWithStack("Can not find partition id %d for table %d", reorgInfo.PhysicalTableID, t.Meta().ID)
}
err = w.addPhysicalTableIndex(p, idx, reorgInfo)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -762,9 +762,9 @@ func (m *Meta) GetDDLReorgHandle(job *model.Job) (startHandle, endHandle, physic
err = errors.Trace(err)
return
}
// endHandle or physicalTableID may be 0, because older version TiDB (without table partition) doesn't store them.
// physicalTableID may be 0, because older version TiDB (without table partition) doesn't store them.
// update them to table's in this case.
if endHandle == 0 || physicalTableID == 0 {
if physicalTableID == 0 {
if job.ReorgMeta != nil {
endHandle = job.ReorgMeta.EndHandle
} else {
Expand Down
8 changes: 7 additions & 1 deletion table/tables/partition.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,13 @@ func (t *partitionedTable) locatePartition(ctx sessionctx.Context, pi *model.Par

// GetPartition returns a Table, which is actually a partition.
func (t *partitionedTable) GetPartition(pid int64) table.PhysicalTable {
return t.partitions[pid]
// Attention, can't simply use `return t.partitions[pid]` here.
// Because A nil of type *partition is a kind of `table.PhysicalTable`
p, ok := t.partitions[pid]
if !ok {
return nil
}
return p
}

// GetPartitionByRow returns a Table, which is actually a Partition.
Expand Down

0 comments on commit d651e27

Please sign in to comment.