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

Tidb SET TIMESTAMP=3147483698 out of range is not compatible with mysql. #31585

Closed
ramanich1 opened this issue Jan 12, 2022 · 2 comments · Fixed by #37249
Closed

Tidb SET TIMESTAMP=3147483698 out of range is not compatible with mysql. #31585

ramanich1 opened this issue Jan 12, 2022 · 2 comments · Fixed by #37249
Assignees
Labels
severity/moderate sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@ramanich1
Copy link
Collaborator

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

SET TIMESTAMP=3147483698;
SELECT UTC_TIMESTAMP();

2. What did you expect to see? (Required)

mysql> SET TIMESTAMP=3147483698;
ERROR 1231 (42000): Variable 'timestamp' can't be set to the value of '3147483698'
-- Retain previously set  timestamp

3. What did you see instead (Required)

mysql> SET TIMESTAMP=3147483698;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> SELECT UTC_TIMESTAMP();
+---------------------+
| UTC_TIMESTAMP()     |
+---------------------+
| 2038-01-19 03:14:07 |
+---------------------+
1 row in set (0.00 sec)
-- Just a warning and resetting timestamp to max value

4. What is your TiDB version? (Required)

| Release Version: v5.4.0-alpha-264-g6efa36df6
Edition: Community
Git Commit Hash: 6efa36df6cff325106f67ecfe3d79816ba37ca6a
Git Branch: master
UTC Build Time: 2021-12-28 02:03:55
GoVersion: go1.17.2
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false |
@ramanich1 ramanich1 added the type/bug The issue is confirmed as a bug. label Jan 12, 2022
@morgo
Copy link
Contributor

morgo commented Jan 12, 2022

This could be seen as a regression of #25686 although at the time, SET TIMESTAMP did not work correctly.

We used to have a feature to by default error for an out of range value, but then changed that to AutoConvert because it was observed MySQL seems to always do this. However, this looks like an exception where AutoConvert is not desired. MySQL is right here to return an error because a warning is dangerous.

See this code:

{Scope: ScopeSession, Name: Timestamp, Value: DefTimestamp, skipInit: true, MinValue: 0, MaxValue: 2147483647, Type: TypeFloat, GetSession: func(s *SessionVars) (string, error) {
if timestamp, ok := s.systems[Timestamp]; ok && timestamp != DefTimestamp {
return timestamp, nil
}
timestamp := s.StmtCtx.GetOrStoreStmtCache(stmtctx.StmtNowTsCacheKey, time.Now()).(time.Time)
return types.ToString(float64(timestamp.UnixNano()) / float64(time.Second))
}},

We could change it to something like the following:

{Scope: ScopeSession, Name: Timestamp, Value: DefTimestamp, skipInit: true, MinValue: 0, MaxValue: math.MaxUint64, Type: TypeFloat, Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) {
	val := tidbOptFloat64(val, DefTimestamp)
	if val > math.MaxInt64 {
		return originalValue, errors.New("out of range")
	}
	return normalizedValue, nil
}}, GetSession: func(s *SessionVars) (string, error) {
		if timestamp, ok := s.systems[Timestamp]; ok && timestamp != DefTimestamp {
			return timestamp, nil
		}
		timestamp := s.StmtCtx.GetOrStoreStmtCache(stmtctx.StmtNowTsCacheKey, time.Now()).(time.Time)
		return types.ToString(float64(timestamp.UnixNano()) / float64(time.Second))
	}},

(Not exactly like this, since the error returned should match the mysql one, and it will need tests etc.)

@morgo
Copy link
Contributor

morgo commented Jan 12, 2022

The other option is to reintroduce AutoConvertOutOfRange as ErrorForOutOfRange. I prefer the validation function, but if we discover other cases where MySQL errors for out of range, we can take this path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/moderate sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants