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

*: query failed after add index / timestamp out-of-range (#28424) #29323

Merged
merged 3 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
44 changes: 41 additions & 3 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4604,10 +4604,48 @@ func (s *testIntegrationSuite) TestIssue27242(c *C) {
tk.MustExec("use test")
tk.MustExec("drop table if exists UK_MU16407")
tk.MustExec("CREATE TABLE UK_MU16407 (COL3 timestamp NULL DEFAULT NULL, UNIQUE KEY U3(COL3));")
defer tk.MustExec("DROP TABLE UK_MU16407")
tk.MustExec(`insert into UK_MU16407 values("1985-08-31 18:03:27");`)
err := tk.ExecToErr(`SELECT COL3 FROM UK_MU16407 WHERE COL3>_utf8mb4'2039-1-19 3:14:40';`)
c.Assert(err, NotNil)
c.Assert(err.Error(), Matches, ".*Incorrect timestamp value.*")
tk.MustExec(`SELECT COL3 FROM UK_MU16407 WHERE COL3>_utf8mb4'2039-1-19 3:14:40';`)
}

func verifyTimestampOutOfRange(tk *testkit.TestKit) {
tk.MustQuery(`select * from t28424 where t != "2038-1-19 3:14:08"`).Sort().Check(testkit.Rows("1970-01-01 00:00:01]\n[2038-01-19 03:14:07"))
tk.MustQuery(`select * from t28424 where t < "2038-1-19 3:14:08"`).Sort().Check(testkit.Rows("1970-01-01 00:00:01]\n[2038-01-19 03:14:07"))
tk.MustQuery(`select * from t28424 where t <= "2038-1-19 3:14:08"`).Sort().Check(testkit.Rows("1970-01-01 00:00:01]\n[2038-01-19 03:14:07"))
tk.MustQuery(`select * from t28424 where t >= "2038-1-19 3:14:08"`).Check(testkit.Rows())
tk.MustQuery(`select * from t28424 where t > "2038-1-19 3:14:08"`).Check(testkit.Rows())
tk.MustQuery(`select * from t28424 where t != "1970-1-1 0:0:0"`).Sort().Check(testkit.Rows("1970-01-01 00:00:01]\n[2038-01-19 03:14:07"))
tk.MustQuery(`select * from t28424 where t < "1970-1-1 0:0:0"`).Check(testkit.Rows())
tk.MustQuery(`select * from t28424 where t <= "1970-1-1 0:0:0"`).Check(testkit.Rows())
tk.MustQuery(`select * from t28424 where t >= "1970-1-1 0:0:0"`).Sort().Check(testkit.Rows("1970-01-01 00:00:01]\n[2038-01-19 03:14:07"))
tk.MustQuery(`select * from t28424 where t > "1970-1-1 0:0:0"`).Sort().Check(testkit.Rows("1970-01-01 00:00:01]\n[2038-01-19 03:14:07"))
}

func (s *testIntegrationSuite) TestIssue28424(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t28424, dt28242")

tk.MustExec(`set time_zone='+00:00'`)
tk.MustExec(`drop table if exists t28424,dt28424`)
tk.MustExec(`create table t28424 (t timestamp)`)
defer tk.MustExec("DROP TABLE t28424")
tk.MustExec(`insert into t28424 values ("2038-01-19 03:14:07"), ("1970-01-01 00:00:01")`)

verifyTimestampOutOfRange(tk)
tk.MustExec(`alter table t28424 add unique index (t)`)
verifyTimestampOutOfRange(tk)
tk.MustExec(`create table dt28424 (dt datetime)`)
defer tk.MustExec("DROP TABLE dt28424")
tk.MustExec(`insert into dt28424 values ("2038-01-19 03:14:07"), ("1970-01-01 00:00:01")`)
tk.MustExec(`insert into dt28424 values ("1969-12-31 23:59:59"), ("1970-01-01 00:00:00"), ("2038-03-19 03:14:08")`)
tk.MustQuery(`select * from t28424 right join dt28424 on t28424.t = dt28424.dt`).Sort().Check(testkit.Rows(
"1970-01-01 00:00:01 1970-01-01 00:00:01]\n" +
"[2038-01-19 03:14:07 2038-01-19 03:14:07]\n" +
"[<nil> 1969-12-31 23:59:59]\n" +
"[<nil> 1970-01-01 00:00:00]\n" +
"[<nil> 2038-03-19 03:14:08"))
}

func (s *testIntegrationSerialSuite) TestTemporaryTableForCte(c *C) {
Expand Down
14 changes: 14 additions & 0 deletions table/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ func handleWrongCharsetValue(ctx sessionctx.Context, col *model.ColumnInfo, str
return err
}

// handleZeroDatetime handles Timestamp/Datetime/Date zero date and invalid dates.
// Currently only called from CastValue.
// returns:
// value (possibly adjusted)
// boolean; true if break error/warning handling in CastValue and return what was returned from this
// error
func handleZeroDatetime(ctx sessionctx.Context, col *model.ColumnInfo, casted types.Datum, str string, tmIsInvalid bool) (types.Datum, bool, error) {
sc := ctx.GetSessionVars().StmtCtx
tm := casted.GetMysqlTime()
Expand Down Expand Up @@ -242,6 +248,14 @@ func handleZeroDatetime(ctx sessionctx.Context, col *model.ColumnInfo, casted ty
sc.AppendWarning(innerErr)
}
return types.NewDatum(zeroV), true, nil
} else if tmIsInvalid && col.Tp == mysql.TypeTimestamp {
// Prevent from being stored! Invalid timestamp!
if mode.HasStrictMode() {
return types.NewDatum(zeroV), true, types.ErrWrongValue.GenWithStackByArgs(zeroT, str)
}
// no strict mode, truncate to 0000-00-00 00:00:00
sc.AppendWarning(types.ErrWrongValue.GenWithStackByArgs(zeroT, str))
return types.NewDatum(zeroV), true, nil
} else if tm.IsZero() || tm.InvalidZero() {
if tm.IsZero() {
// Don't care NoZeroDate mode if time val is invalid.
Expand Down
3 changes: 2 additions & 1 deletion types/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,8 @@ func (d *Datum) convertToMysqlTimestamp(sc *stmtctx.StatementContext, target *Fi
case KindMysqlTime:
t, err = d.GetMysqlTime().Convert(sc, target.Tp)
if err != nil {
ret.SetMysqlTime(ZeroTimestamp)
// t might be an invalid Timestamp, but should still be comparable, since same representation (KindMysqlTime)
ret.SetMysqlTime(t)
return ret, errors.Trace(ErrWrongValue.GenWithStackByArgs(TimestampStr, t.String()))
}
t, err = t.RoundFrac(sc, fsp)
Expand Down
3 changes: 3 additions & 0 deletions util/ranger/ranger.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ func convertPoint(sctx sessionctx.Context, point *point, tp *types.FieldType) (*
// Ignore the types.ErrOverflow when we convert TypeNewDecimal values.
// A trimmed valid boundary point value would be returned then. Accordingly, the `excl` of the point
// would be adjusted. Impossible ranges would be skipped by the `validInterval` call later.
} else if point.value.Kind() == types.KindMysqlTime && tp.Tp == mysql.TypeTimestamp && terror.ErrorEqual(err, types.ErrWrongValue) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only case that return types.ErrWrongValue and can be saftly ignored?
If in some other cases ErrWrongValue is also returned, and we ignore the error unexpectedly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so.

From what I can see, the ConvertTo(sc, tp) with Kind() == types.KindMysqlTime and tp.Tp == mysql.TypeTimestamp can only end up in the part of types/datum.go above, that I also changed in this PR, so I cannot see any other case that would end up being ignored here.

I.e. it is only points of MysqlTime converted to Timestamp that would generate an error in GetMysqlTime().Convert() that would be matched here.

// See issue #28424: query failed after add index
// Ignore conversion from Date[Time] to Timestamp since it must be either out of range or impossible date, which will not match a point select
} else if tp.Tp == mysql.TypeEnum && terror.ErrorEqual(err, types.ErrTruncated) {
// Ignore the types.ErrorTruncated when we convert TypeEnum values.
// We should cover Enum upper overflow, and convert to the biggest value.
Expand Down