ddl:support add index rollback of partitioned table #7437
Conversation
ciscoxll
force-pushed
the
index-roll-back
branch
from
bed2fa5
to
a63cf4c
Compare
| @@ -268,12 +268,23 @@ func insertJobIntoDeleteRangeTable(ctx sessionctx.Context, job *model.Job) error | |||
| case model.ActionAddIndex: | |||
| tableID := job.TableID | |||
| var indexID int64 | |||
| if err := job.DecodeArgs(&indexID); err != nil { | |||
| var partitionIDs []int64 | |||
| if err := job.DecodeArgs(&indexID, &partitionIDs); err != nil { | |||
There was a problem hiding this comment.
Will it break the backward compatibility?
There was a problem hiding this comment.
@jackysp Just adding parameters does not break compatibility.
| if err != nil { | ||
| checkErr = errors.Trace(err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Is this function the same as the hook.OnJobUpdatedExported in TestCancelAddIndex?
There was a problem hiding this comment.
@zimulala Yes, they are the same.
There was a problem hiding this comment.
@ciscoxll This code is more complicated, could this be extracted into a function?
|
LGTM |
tiancaiamao
added
status/LGT2
| startKey := tablecodec.EncodeTableIndexPrefix(pid, indexID) | ||
| endKey := tablecodec.EncodeTableIndexPrefix(pid, indexID+1) | ||
| if err := doInsert(s, job.ID, indexID, startKey, endKey, now); err != nil { | ||
| return errors.Trace(err) |
There was a problem hiding this comment.
Using break here can help us to save else.
There was a problem hiding this comment.
@XuHuaiyu It feels like it doesn't have to be modified, these are two different kinds of logic.
| func (s *testDBSuite) TestPartitionCancelAddIndex(c *C) { | ||
| s.tk = testkit.NewTestKit(c, s.store) | ||
| s.mustExec(c, "use test_db") | ||
| s.mustExec(c, "set @@session.tidb_enable_table_partition=1;") |
There was a problem hiding this comment.
should we set @@session.tidb_enable_table_partition=0; at the end of this function.
There was a problem hiding this comment.
It's should be OK to set @@session.tidb_enable_table_partition=1; for any test code. @XuHuaiyu
We'll enable table partition by default later.
|
@zimulala PTAL. |
| s.dom.DDL().(ddl.DDLForTest).SetHook(callback) | ||
| } | ||
|
|
||
| func backgroundExecOnJobUpdatedExported(c *C, s *testDBSuite, hook *ddl.TestDDLCallback, checkErr error) (func(*model.Job), *model.IndexInfo) { |
There was a problem hiding this comment.
Cloud we use it in "TestCancelAddIndex"? And hold the comment in original “TestCancelAddIndex”.
|
@zimulala PTAL . |
ciscoxll
force-pushed
the
index-roll-back
branch
from
668d754
to
4a0b0f2
Compare
|
/run-all-tests |
1 similar comment
|
/run-all-tests |
zimulala
added
status/LGT3
What problem does this PR solve?
rollback.What is changed and how it works?
Drop indexmainly deletesindexInfo.delete range.Check List
Tests
PTAL @tiancaiamao @winkyao @zimulala .