Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ddl: fix "wrong warning messages when running some DDL jobs" #38761

Merged
merged 17 commits into from
Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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'"))
}