Skip to content

expression: Fix the issue that cast str as datetime with time_zone results in loss of precision #50958

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.

Sign up for GitHub

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

Merged
ti-chi-bot merged 7 commits into from
Feb 6, 2024
Merged

expression: Fix the issue that cast str as datetime with time_zone results in loss of precision #50958

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d8144f2
add one of ut
SeaRise Feb 4, 2024
68f8f8a
fix
SeaRise Feb 4, 2024
3bb43c4
u
SeaRise Feb 4, 2024
86072f5
Update time_test.go
SeaRise Feb 5, 2024
7aabcdc
Update time.go
SeaRise Feb 5, 2024
9744f94
Update pkg/types/time_test.go
SeaRise Feb 5, 2024
cd038bc
update
SeaRise Feb 5, 2024
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: 5 additions & 2 deletions pkg/types/time.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,10 @@ func GetFsp(s string) int {

// GetFracIndex finds the last '.' for get fracStr, index = -1 means fracStr not found.
// but for format like '2019.01.01 00:00:00', the index should be -1.
// It will not be affected by the time zone suffix. For format like '2020-01-01 12:00:00.123456+05:00', the index should be 19.
//
// It will not be affected by the time zone suffix.
// For format like '2020-01-01 12:00:00.123456+05:00' and `2020-01-01 12:00:00.123456-05:00`, the index should be 19.
// related issue https://github.com/pingcap/tidb/issues/35291 and https://github.com/pingcap/tidb/issues/49555
func GetFracIndex(s string) (index int) {
tzIndex, _, _, _, _ := GetTimezone(s)
var end int
Expand All @@ -587,7 +590,7 @@ func GetFracIndex(s string) (index int) {
}
index = -1
for i := end; i >= 0; i-- {
if unicode.IsPunct(rune(s[i])) {
if s[i] != '+' && s[i] != '-' && unicode.IsPunct(rune(s[i])) {
Copy link
Copy Markdown
Contributor

@Defined2014 Defined2014 Feb 5, 2024

Choose a reason for hiding this comment

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

We have some functions in types/time.go and types/helper.go, like isValidSeparator and isPunctuation. Not sure could we directly use them.

Copy link
Copy Markdown
Contributor Author

@SeaRise SeaRise Feb 5, 2024

Choose a reason for hiding this comment

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

It looks like there is no function available.
BTW, I replace unicode.IsPunct by isPunctuation.

Copy link
Copy Markdown
Contributor

@Defined2014 Defined2014 Feb 5, 2024

Choose a reason for hiding this comment

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

Ok, just a question more than suggestion.

Seems punctuation in different language has its own meaning. I think isPunctuation function is followed std::ispunct which may different with unicode.IsPunct. Not sure which one is correct.

Copy link
Copy Markdown
Contributor Author

@SeaRise SeaRise Feb 5, 2024

Choose a reason for hiding this comment

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

As isPunctuation was used consistently in time.go, I have replaced it here. It seems that in time.go, either all function should use isPunctuation, or all instances should use unicode.IsPunct.

Copy link
Copy Markdown
Contributor Author

@SeaRise SeaRise Feb 5, 2024

Choose a reason for hiding this comment

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

I believe replacing it with isPunctuation is safe here, as this code snippet was introduced by #9933 to handle the case like 2019.01.01 00:00:00.

if s[i] == '.' {
index = i
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/types/time_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ func TestDateTime(t *testing.T) {

// For issue 35291
{"2020-01-01 12:00:00.123456+05:00", "2020-01-01 07:00:00.123456"},
// For issue 49555
{"2020-01-01 12:00:00.123456-05:00", "2020-01-01 07:00:00.123456"},
Comment thread
SeaRise marked this conversation as resolved.
Outdated
}

for _, test := range table {
Expand Down Expand Up @@ -1902,6 +1904,8 @@ func TestGetFracIndex(t *testing.T) {
{"2019.01.01 00:00:00", -1},
{"2019.01.01 00:00:00.1", 19},
{"12345.6", 5},
{"2020-01-01 12:00:00.123456 +0600 PST", 19},
{"2020-01-01 12:00:00.123456 -0600 PST", 19},
}
for _, testCase := range testCases {
index := types.GetFracIndex(testCase.str)
Expand Down
10 changes: 10 additions & 0 deletions tests/integrationtest/r/expression/cast.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ select cast(col4 as time(31)) from t where col1 is null;
Error 1426 (42000): Too big precision 31 specified for column 'CAST'. Maximum is 6.
select cast(col5 as time(31)) from t where col1 is null;
Error 1426 (42000): Too big precision 31 specified for column 'CAST'. Maximum is 6.
drop table if exists t;
create table t(a varchar(50));
insert into t values ('2020-01-01 12:00:00.123456 +0600 PST');
insert into t values ('2020-01-01 12:00:00.123456 -0600 PST');
insert into t values ('2020-01-01 12:00:00.123456');
select cast(a as datetime(3)) from t;
cast(a as datetime(3))
2020-01-01 12:00:00.123
2020-01-01 12:00:00.123
2020-01-01 12:00:00.123
drop table if exists t1;
create table t1 (c1 text);
insert into t1 values ('a');
Expand Down
6 changes: 6 additions & 0 deletions tests/integrationtest/t/expression/cast.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ select cast(col3 as time(31)) from t where col1 is null;
select cast(col4 as time(31)) from t where col1 is null;
-- error 1426
select cast(col5 as time(31)) from t where col1 is null;
drop table if exists t;
create table t(a varchar(50));
insert into t values ('2020-01-01 12:00:00.123456 +0600 PST');
insert into t values ('2020-01-01 12:00:00.123456 -0600 PST');
insert into t values ('2020-01-01 12:00:00.123456');
select cast(a as datetime(3)) from t;

# TestCastErrMsg
drop table if exists t1;
Expand Down
⚡