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: disallow change columns from zero int to time #25728

Merged
merged 10 commits into from
Nov 30, 2021
2 changes: 2 additions & 0 deletions ddl/backfilling.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ func (w *worker) writePhysicalTableRecord(t table.PhysicalTable, bfWorkerType ba
backfillWorkers = append(backfillWorkers, idxWorker.backfillWorker)
go idxWorker.backfillWorker.run(reorgInfo.d, idxWorker, job)
case typeUpdateColumnWorker:
// Setting InCreateOrAlterStmt tells the difference between SELECT casting and ALTER COLUMN casting.
sessCtx.GetSessionVars().StmtCtx.InCreateOrAlterStmt = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might we add a brief comment here (cast diff in select & alter)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tangenta Please update the comment according to the address, thanks.

updateWorker := newUpdateColumnWorker(sessCtx, w, i, t, oldColInfo, colInfo, decodeColMap, reorgInfo.ReorgMeta.SQLMode)
updateWorker.priority = job.Priority
backfillWorkers = append(backfillWorkers, updateWorker.backfillWorker)
Expand Down
46 changes: 46 additions & 0 deletions ddl/column_type_change_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2128,6 +2128,52 @@ func (s *testColumnTypeChangeSuite) TestCastToTimeStampDecodeError(c *C) {
tk.MustQuery("select timestamp(cast('1000-11-11 12-3-1' as date));").Check(testkit.Rows("1000-11-11 00:00:00"))
}

// https://github.com/pingcap/tidb/issues/25285.
func (s *testColumnTypeChangeSuite) TestCastFromZeroIntToTimeError(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test;")

prepare := func() {
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int);")
tk.MustExec("insert into t values (0);")
}
const errCodeNone = -1
testCases := []struct {
sqlMode string
errCode int
}{
{"STRICT_TRANS_TABLES", mysql.ErrTruncatedWrongValue},
{"STRICT_ALL_TABLES", mysql.ErrTruncatedWrongValue},
{"NO_ZERO_IN_DATE", errCodeNone},
{"NO_ZERO_DATE", errCodeNone},
{"ALLOW_INVALID_DATES", errCodeNone},
{"", errCodeNone},
}
for _, tc := range testCases {
prepare()
tk.MustExec(fmt.Sprintf("set @@sql_mode = '%s';", tc.sqlMode))
if tc.sqlMode == "NO_ZERO_DATE" {
tk.MustQuery(`select date(0);`).Check(testkit.Rows("<nil>"))
} else {
tk.MustQuery(`select date(0);`).Check(testkit.Rows("0000-00-00"))
}
tk.MustQuery(`select time(0);`).Check(testkit.Rows("00:00:00"))
if tc.errCode == errCodeNone {
tk.MustExec("alter table t modify column a date;")
prepare()
tk.MustExec("alter table t modify column a datetime;")
prepare()
tk.MustExec("alter table t modify column a timestamp;")
} else {
tk.MustGetErrCode("alter table t modify column a date;", mysql.ErrTruncatedWrongValue)
tk.MustGetErrCode("alter table t modify column a datetime;", mysql.ErrTruncatedWrongValue)
tk.MustGetErrCode("alter table t modify column a timestamp;", mysql.ErrTruncatedWrongValue)
}
}
tk.MustExec("drop table if exists t;")
}

func (s *testColumnTypeChangeSuite) TestChangeFromTimeToYear(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test;")
Expand Down
9 changes: 3 additions & 6 deletions ddl/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5474,8 +5474,7 @@ func (s *testDBSuite1) TestModifyColumnTime_YearToDate(c *C) {
{"year", `"69"`, "date", "", errno.ErrTruncatedWrongValue},
{"year", `"70"`, "date", "", errno.ErrTruncatedWrongValue},
{"year", `"99"`, "date", "", errno.ErrTruncatedWrongValue},
// MySQL will get "Data truncation: Incorrect date value: '0000'", but TiDB treat 00 as valid datetime.
{"year", `00`, "date", "0000-00-00", 0},
{"year", `00`, "date", "", errno.ErrTruncatedWrongValue},
{"year", `69`, "date", "", errno.ErrTruncatedWrongValue},
{"year", `70`, "date", "", errno.ErrTruncatedWrongValue},
{"year", `99`, "date", "", errno.ErrTruncatedWrongValue},
Expand All @@ -5492,8 +5491,7 @@ func (s *testDBSuite1) TestModifyColumnTime_YearToDatetime(c *C) {
{"year", `"69"`, "datetime", "", errno.ErrTruncatedWrongValue},
{"year", `"70"`, "datetime", "", errno.ErrTruncatedWrongValue},
{"year", `"99"`, "datetime", "", errno.ErrTruncatedWrongValue},
// MySQL will get "Data truncation: Incorrect date value: '0000'", but TiDB treat 00 as valid datetime.
{"year", `00`, "datetime", "0000-00-00 00:00:00", 0},
{"year", `00`, "datetime", "", errno.ErrTruncatedWrongValue},
{"year", `69`, "datetime", "", errno.ErrTruncatedWrongValue},
{"year", `70`, "datetime", "", errno.ErrTruncatedWrongValue},
{"year", `99`, "datetime", "", errno.ErrTruncatedWrongValue},
Expand All @@ -5510,8 +5508,7 @@ func (s *testDBSuite1) TestModifyColumnTime_YearToTimestamp(c *C) {
{"year", `"69"`, "timestamp", "", errno.ErrTruncatedWrongValue},
{"year", `"70"`, "timestamp", "", errno.ErrTruncatedWrongValue},
{"year", `"99"`, "timestamp", "", errno.ErrTruncatedWrongValue},
// MySQL will get "Data truncation: Incorrect date value: '0000'", but TiDB treat 00 as valid datetime.
{"year", `00`, "timestamp", "0000-00-00 00:00:00", 0},
{"year", `00`, "timestamp", "", errno.ErrTruncatedWrongValue},
{"year", `69`, "timestamp", "", errno.ErrTruncatedWrongValue},
{"year", `70`, "timestamp", "", errno.ErrTruncatedWrongValue},
{"year", `99`, "timestamp", "", errno.ErrTruncatedWrongValue},
Expand Down
2 changes: 1 addition & 1 deletion expression/builtin_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func (b *builtinDateSig) evalTime(row chunk.Row) (types.Time, bool, error) {
return types.ZeroTime, true, handleInvalidTimeError(b.ctx, err)
}

if expr.IsZero() {
if expr.IsZero() && b.ctx.GetSessionVars().SQLMode.HasNoZeroDateMode() {
return types.ZeroTime, true, handleInvalidTimeError(b.ctx, types.ErrWrongValue.GenWithStackByArgs(types.DateTimeStr, expr.String()))
}

Expand Down
2 changes: 1 addition & 1 deletion expression/builtin_time_vec.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (b *builtinDateSig) vecEvalTime(input *chunk.Chunk, result *chunk.Column) e
if result.IsNull(i) {
continue
}
if times[i].IsZero() {
if times[i].IsZero() && b.ctx.GetSessionVars().SQLMode.HasNoZeroDateMode() {
if err := handleInvalidTimeError(b.ctx, types.ErrWrongValue.GenWithStackByArgs(types.DateTimeStr, times[i].String())); err != nil {
return err
}
Expand Down
13 changes: 12 additions & 1 deletion types/time.go
Original file line number Diff line number Diff line change
Expand Up @@ -1987,7 +1987,18 @@ func ParseTimeFromYear(sc *stmtctx.StatementContext, year int64) (Time, error) {
func ParseTimeFromNum(sc *stmtctx.StatementContext, num int64, tp byte, fsp int8) (Time, error) {
// MySQL compatibility: 0 should not be converted to null, see #11203
if num == 0 {
return NewTime(ZeroCoreTime, tp, DefaultFsp), nil
zt := NewTime(ZeroCoreTime, tp, DefaultFsp)
if sc != nil && sc.InCreateOrAlterStmt && !sc.TruncateAsWarning {
switch tp {
case mysql.TypeTimestamp:
return zt, ErrTruncatedWrongVal.GenWithStackByArgs(TimestampStr, "0")
case mysql.TypeDate:
return zt, ErrTruncatedWrongVal.GenWithStackByArgs(DateStr, "0")
case mysql.TypeDatetime:
return zt, ErrTruncatedWrongVal.GenWithStackByArgs(DateTimeStr, "0")
}
}
return zt, nil
}
fsp, err := CheckFsp(int(fsp))
if err != nil {
Expand Down