From 23796a46a0710f1cf25ae631d3e058d9f197c844 Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Mon, 11 Dec 2023 11:58:48 +0800 Subject: [PATCH] ddl: fix recover table by JobID bug when JobID is set to 0 tidb-server panic (#46343) (#48086) close pingcap/tidb#46296 --- executor/ddl.go | 7 ++++--- executor/recover_test.go | 5 +++++ parser/ast/ddl.go | 8 ++++---- parser/parser_test.go | 1 + 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/executor/ddl.go b/executor/ddl.go index 7dc2ab47de971..5b7556ef7b97e 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -392,10 +392,11 @@ func (e *DDLExec) executeRecoverTable(s *ast.RecoverTableStmt) error { var job *model.Job var err error var tblInfo *model.TableInfo - if s.JobID != 0 { - job, tblInfo, err = e.getRecoverTableByJobID(s, dom) - } else { + // Let check table first. Related isssue #46296. + if s.Table != nil { job, tblInfo, err = e.getRecoverTableByTableName(s.Table) + } else { + job, tblInfo, err = e.getRecoverTableByJobID(s, dom) } if err != nil { return err diff --git a/executor/recover_test.go b/executor/recover_test.go index 0b8c342b1111c..27d20c24a1600 100644 --- a/executor/recover_test.go +++ b/executor/recover_test.go @@ -92,6 +92,11 @@ func TestRecoverTable(t *testing.T) { err := tk.ExecToErr(fmt.Sprintf("recover table by job %d", 10000000)) require.Error(t, err) + // recover table by zero JobID. + // related issue: https://github.com/pingcap/tidb/issues/46296 + err = tk.ExecToErr(fmt.Sprintf("recover table by job %d", 0)) + require.Error(t, err) + // Disable GC by manual first, then after recover table, the GC enable status should also be disabled. require.NoError(t, gcutil.DisableGC(tk.Session())) diff --git a/parser/ast/ddl.go b/parser/ast/ddl.go index 03f07ac194ff4..8f2ad83946d5d 100644 --- a/parser/ast/ddl.go +++ b/parser/ast/ddl.go @@ -4228,16 +4228,16 @@ type RecoverTableStmt struct { // Restore implements Node interface. func (n *RecoverTableStmt) Restore(ctx *format.RestoreCtx) error { ctx.WriteKeyWord("RECOVER TABLE ") - if n.JobID != 0 { - ctx.WriteKeyWord("BY JOB ") - ctx.WritePlainf("%d", n.JobID) - } else { + if n.Table != nil { if err := n.Table.Restore(ctx); err != nil { return errors.Annotate(err, "An error occurred while splicing RecoverTableStmt Table") } if n.JobNum > 0 { ctx.WritePlainf(" %d", n.JobNum) } + } else { + ctx.WriteKeyWord("BY JOB ") + ctx.WritePlainf("%d", n.JobID) } return nil } diff --git a/parser/parser_test.go b/parser/parser_test.go index 91ee4c4b1c68c..2b4847afb72d7 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -3311,6 +3311,7 @@ func TestDDL(t *testing.T) { {"recover table by job 11", true, "RECOVER TABLE BY JOB 11"}, {"recover table by job 11,12,13", false, ""}, {"recover table by job", false, ""}, + {"recover table by job 0", true, "RECOVER TABLE BY JOB 0"}, {"recover table t1", true, "RECOVER TABLE `t1`"}, {"recover table t1,t2", false, ""}, {"recover table ", false, ""},