Skip to content

Commit

Permalink
ddl: fix "wrong warning messages when running some DDL jobs" (#38761)
Browse files Browse the repository at this point in the history
close #38699
  • Loading branch information
wxbty authored Nov 17, 2022
1 parent 533fd31 commit 077cde7
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 42 deletions.
7 changes: 6 additions & 1 deletion ddl/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1094,8 +1094,13 @@ func (d *ddl) DoDDLJob(ctx sessionctx.Context, job *model.Job) error {
logutil.BgLogger().Info("[ddl] DDL warnings doesn't match the warnings count", zap.Int64("jobID", jobID))
} else {
for key, warning := range historyJob.ReorgMeta.Warnings {
for j := int64(0); j < historyJob.ReorgMeta.WarningsCount[key]; j++ {
keyCount := historyJob.ReorgMeta.WarningsCount[key]
if keyCount == 1 {
ctx.GetSessionVars().StmtCtx.AppendWarning(warning)
} else {
newMsg := fmt.Sprintf("%d warnings with this error code, first warning: "+warning.GetMsg(), keyCount)
newWarning := dbterror.ClassTypes.Synthesize(terror.ErrCode(warning.Code()), newMsg)
ctx.GetSessionVars().StmtCtx.AppendWarning(newWarning)
}
}
}
Expand Down
29 changes: 29 additions & 0 deletions ddl/failtest/fail_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"math/rand"
"strings"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -532,6 +533,34 @@ func TestModifyColumn(t *testing.T) {
tk.MustExec("drop table t, t2, t3, t4, t5")
}

func TestIssue38699(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)

//Test multi records
tk.MustExec("USE test")
tk.MustExec("set sql_mode=''")
tk.MustExec("DROP TABLE IF EXISTS t;")
tk.MustExec("CREATE TABLE t (a int)")
tk.MustExec("insert into t values (1000000000), (2000000)")
tk.MustExec("alter table t modify a tinyint")
result := tk.MustQuery("show warnings")
require.Len(t, result.Rows(), 1)
result.CheckWithFunc(testkit.Rows("Warning 1690 2 warnings with this error code"), func(actual []string, expected []interface{}) bool {
//Check if it starts with x warning(s)
return strings.EqualFold(actual[0], expected[0].(string)) && strings.EqualFold(actual[1], expected[1].(string)) && strings.HasPrefix(actual[2], expected[2].(string))
})

//Test single record
tk.MustExec("DROP TABLE IF EXISTS t;")
tk.MustExec("CREATE TABLE t (a int)")
tk.MustExec("insert into t values (1000000000)")
tk.MustExec("alter table t modify a tinyint")
result = tk.MustQuery("show warnings")
require.Len(t, result.Rows(), 1)
result.Check(testkit.Rows("Warning 1690 constant 1000000000 overflows tinyint"))
}

func TestPartitionAddPanic(t *testing.T) {
s := createFailDBSuite(t)
tk := testkit.NewTestKit(t, s.store)
Expand Down
45 changes: 4 additions & 41 deletions ddl/modify_column_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -813,10 +813,7 @@ func TestModifyColumnTypeWithWarnings(t *testing.T) {
// 111.22 will be truncated the fraction .22 as .2 with truncated warning for each row.
tk.MustExec("alter table t modify column a decimal(4,1)")
// there should 4 rows of warnings corresponding to the origin rows.
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1292 Truncated incorrect DECIMAL value: '111.22'",
"Warning 1292 Truncated incorrect DECIMAL value: '111.22'",
"Warning 1292 Truncated incorrect DECIMAL value: '111.22'",
"Warning 1292 Truncated incorrect DECIMAL value: '111.22'"))
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1292 4 warnings with this error code, first warning: Truncated incorrect DECIMAL value: '111.22'"))

// Test the strict warnings is treated as errors under the strict mode.
tk.MustExec("drop table if exists t")
Expand All @@ -829,15 +826,13 @@ func TestModifyColumnTypeWithWarnings(t *testing.T) {
// Test the strict warnings is treated as warnings under the non-strict mode.
tk.MustExec("set @@sql_mode=\"\"")
tk.MustExec("alter table t modify column a decimal(3,1)")
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1690 DECIMAL value is out of range in '(3, 1)'",
"Warning 1690 DECIMAL value is out of range in '(3, 1)'",
"Warning 1690 DECIMAL value is out of range in '(3, 1)'"))
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1690 3 warnings with this error code, first warning: DECIMAL value is out of range in '(3, 1)'"))
}

// TestModifyColumnTypeWhenInterception is to test modifying column type with warnings intercepted by
// reorg timeout, not owner error and so on.
func TestModifyColumnTypeWhenInterception(t *testing.T) {
store, dom := testkit.CreateMockStoreAndDomain(t)
store, _ := testkit.CreateMockStoreAndDomain(t)

tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
Expand All @@ -858,42 +853,10 @@ func TestModifyColumnTypeWhenInterception(t *testing.T) {
// Make the regions scale like: [1, 1024), [1024, 2048), [2048, 3072), [3072, 4096]
tk.MustQuery("split table t between(0) and (4096) regions 4")

d := dom.DDL()
hook := &ddl.TestDDLCallback{}
var checkMiddleWarningCount bool
var checkMiddleAddedCount bool
// Since the `DefTiDBDDLReorgWorkerCount` is 4, every worker will be assigned with one region
// for the first time. Here we mock the insert failure/reorg timeout in region [2048, 3072)
// which will lead next handle be set to 2048 and partial warnings be stored into ddl job.
// Since the existence of reorg batch size, only the last reorg batch [2816, 3072) of kv
// range [2048, 3072) fail to commit, the rest of them all committed successfully. So the
// addedCount and warnings count in the job are all equal to `4096 - reorg batch size`.
// In the next round of this ddl job, the last reorg batch will be finished.
var middleWarningsCount = int64(defaultBatchSize*4 - defaultReorgBatchSize)
onJobUpdatedExportedFunc := func(job *model.Job) {
if job.SchemaState == model.StateWriteReorganization || job.SnapshotVer != 0 {
if len(job.ReorgMeta.WarningsCount) == len(job.ReorgMeta.Warnings) {
for _, v := range job.ReorgMeta.WarningsCount {
if v == middleWarningsCount {
checkMiddleWarningCount = true
}
}
}
if job.RowCount == middleWarningsCount {
checkMiddleAddedCount = true
}
}
}
hook.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
d.SetHook(hook)
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/ddl/MockReorgTimeoutInOneRegion", `return(true)`))
defer func() {
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/MockReorgTimeoutInOneRegion"))
}()
tk.MustExec("alter table t modify column b decimal(3,1)")
require.True(t, checkMiddleWarningCount)
require.True(t, checkMiddleAddedCount)

res := tk.MustQuery("show warnings")
require.Len(t, res.Rows(), count)
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1292 4096 warnings with this error code, first warning: Truncated incorrect DECIMAL value: '11.22'"))
}

0 comments on commit 077cde7

Please sign in to comment.