*: support check an index #5932
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alivxxx
reviewed
Mar 2, 2018
parser/parser_test.go
Outdated
| @@ -403,6 +403,7 @@ func (s *testParserSuite) TestDMLStmt(c *C) { | |||
| {"admin show ddl;", true}, | |||
| {"admin show ddl jobs;", true}, | |||
| {"admin check table t1, t2;", true}, | |||
| {"admin check index idx;", true}, | |||
There was a problem hiding this comment.
So the idx is the table name, and index name is empty?
| break | ||
| } | ||
| } | ||
| if idx == nil { |
There was a problem hiding this comment.
Do we need to check if the index state is public?
executor/executor.go
Outdated
| } | ||
| err := e.run(ctx) | ||
| e.done = true | ||
| return errors.Trace(err) |
There was a problem hiding this comment.
Can we just call e.Next or extract a function?
winoros
reviewed
Mar 2, 2018
executor/executor.go
Outdated
| } | ||
|
|
||
| for { | ||
| row, err := e.src.Next(ctx) |
zz-jason
reviewed
Mar 2, 2018
executor/executor.go
Outdated
| dbName string | ||
| tableName string | ||
| idxName string | ||
| src Executor |
There was a problem hiding this comment.
s/Executor/*IndexLookUpExecutor/ is more readable ?
shenli
reviewed
Mar 2, 2018
executor/distsql.go
Outdated
| @@ -878,6 +886,10 @@ func (w *tableWorker) executeTask(ctx context.Context, task *lookupTableTask) { | |||
| } | |||
| sort.Sort(task) | |||
| } | |||
|
|
|||
| if w.isCheckOp && handleCnt != len(task.rows) { | |||
| err = errors.Errorf("handle count %d isn't equal to value count %d", handleCnt, len(task.rows)) | |||
There was a problem hiding this comment.
Add the missing handle and corresponding index value in the error message.
executor/executor.go
Outdated
| @@ -409,6 +409,70 @@ func (e *CheckTableExec) run(ctx context.Context) error { | |||
| return nil | |||
| } | |||
|
|
|||
| // CheckIndexExec represents the executor of check an index. | |||
executor/executor.go
Outdated
| @@ -409,6 +409,70 @@ func (e *CheckTableExec) run(ctx context.Context) error { | |||
| return nil | |||
| } | |||
|
|
|||
| // CheckIndexExec represents the executor of check an index. | |||
| // It is built from the "admin check index" statement, and it checks if the | |||
There was a problem hiding this comment.
it checks the consistency of the index data with the records of the table.
zz-jason
reviewed
Mar 3, 2018
executor/executor.go
Outdated
| if err != nil { | ||
| return errors.Trace(err) | ||
| } | ||
| cnt := 0 |
|
PTAL @zz-jason |
shenli
added
the
Priority/P1
|
LGTM |
|
/run-all-tests |
shenli
added
status/LGT2
zz-jason
approved these changes
Mar 5, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using
IndexLoopUpReaderto supportadmin check indexoption.In next PR I will handle the case below：