[SPARK-32559][SQL]Fix the trim logic in UTF8String.toInt/toLong did't handle non-ASCII characters correctly #29375
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@cloud-fan @yaooqinn @gengliangwang Could you please help review this? |
yaooqinn
reviewed
Aug 6, 2020
common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
Outdated
Show resolved
Hide resolved
WangGuangxin
force-pushed
the
cast-bugfix
branch
from
cf674a7 to
8509cff
Compare
Member
|
retest this please |
WangGuangxin
force-pushed
the
cast-bugfix
branch
from
8509cff to
a0adf3c
Compare
Contributor
|
ok to test |
…aracters correctly
WangGuangxin
force-pushed
the
cast-bugfix
branch
from
a0adf3c to
5db4b41
Compare
WangGuangxin
changed the title
|
Test build #127144 has finished for PR 29375 at commit
|
|
Test build #127148 has finished for PR 29375 at commit
|
Contributor
|
thanks, merging to master! |
Contributor
|
@WangGuangxin can you open a backport PR for 3.0? |
Contributor
Author
sure |
WangGuangxin
added a commit
to WangGuangxin/spark
that referenced
this pull request
Aug 7, 2020
…t handle non-ASCII characters correctly The trim logic in Cast expression introduced in apache#26622 trim non-ASCII characters unexpectly. Before this patch  After this patch  The behavior described above doesn't make sense, and also doesn't consistent with the behavior when cast a string to double/float, as well as doesn't consistent with the behavior of Hive Yes Added more UT Closes apache#29375 from WangGuangxin/cast-bugfix. Authored-by: wangguangxin.cn <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
srowen
reviewed
Aug 9, 2020
| select cast(' 1' as float); | ||
| select cast(' 1 ' as DOUBLE); | ||
| select cast('1.0 ' as DEC); | ||
| select cast('1中文' as tinyint); |
Member
There was a problem hiding this comment.
This is purely to educate me, but those characters are considered whitespace?
Contributor
There was a problem hiding this comment.
This kind of character needs multiple bytes, so getByte(s) <= ' ' may not work.
dongjoon-hyun
pushed a commit
that referenced
this pull request
Aug 9, 2020
… did't handle non-ASCII characters correctly ### What changes were proposed in this pull request? This is a backport of #29375 The trim logic in Cast expression introduced in #26622 trim non-ASCII characters unexpectly. Before this patch  After this patch  ### Why are the changes needed? The behavior described above doesn't make sense, and also doesn't consistent with the behavior when cast a string to double/float, as well as doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? Added more UT Closes #29393 from WangGuangxin/cast-bugfix-branch-3.0. Authored-by: wangguangxin.cn <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
cloud-fan
pushed a commit
that referenced
this pull request
Jul 13, 2021
…r change of trimming characters for cast
### What changes were proposed in this pull request?
This PR modifies comment for `UTF8String.trimAll` and`sql-migration-guide.mld`.
The comment for `UTF8String.trimAll` says like as follows.
```
Trims whitespaces ({literal <=} ASCII 32) from both ends of this string.
```
Similarly, `sql-migration-guide.md` mentions about the behavior of `cast` like as follows.
```
In Spark 3.0, when casting string value to integral types(tinyint, smallint, int and bigint),
datetime types(date, timestamp and interval) and boolean type,
the leading and trailing whitespaces (<= ASCII 32) will be trimmed before converted to these type values,
for example, `cast(' 1\t' as int)` results `1`, `cast(' 1\t' as boolean)` results `true`,
`cast('2019-10-10\t as date)` results the date value `2019-10-10`.
In Spark version 2.4 and below, when casting string to integrals and booleans,
it does not trim the whitespaces from both ends; the foregoing results is `null`,
while to datetimes, only the trailing spaces (= ASCII 32) are removed.
```
But SPARK-32559 (#29375) changed the behavior and only whitespace ASCII characters will be trimmed since Spark 3.0.1.
### Why are the changes needed?
To follow the previous change.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Confirmed the document built by the following command.
```
SKIP_API=1 bundle exec jekyll build
```
Closes #33287 from sarutak/fix-utf8string-trim-issue.
Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
cloud-fan
pushed a commit
that referenced
this pull request
Jul 13, 2021
…r change of trimming characters for cast
### What changes were proposed in this pull request?
This PR modifies comment for `UTF8String.trimAll` and`sql-migration-guide.mld`.
The comment for `UTF8String.trimAll` says like as follows.
```
Trims whitespaces ({literal <=} ASCII 32) from both ends of this string.
```
Similarly, `sql-migration-guide.md` mentions about the behavior of `cast` like as follows.
```
In Spark 3.0, when casting string value to integral types(tinyint, smallint, int and bigint),
datetime types(date, timestamp and interval) and boolean type,
the leading and trailing whitespaces (<= ASCII 32) will be trimmed before converted to these type values,
for example, `cast(' 1\t' as int)` results `1`, `cast(' 1\t' as boolean)` results `true`,
`cast('2019-10-10\t as date)` results the date value `2019-10-10`.
In Spark version 2.4 and below, when casting string to integrals and booleans,
it does not trim the whitespaces from both ends; the foregoing results is `null`,
while to datetimes, only the trailing spaces (= ASCII 32) are removed.
```
But SPARK-32559 (#29375) changed the behavior and only whitespace ASCII characters will be trimmed since Spark 3.0.1.
### Why are the changes needed?
To follow the previous change.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Confirmed the document built by the following command.
```
SKIP_API=1 bundle exec jekyll build
```
Closes #33287 from sarutak/fix-utf8string-trim-issue.
Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 57a4f31)
Signed-off-by: Wenchen Fan <[email protected]>
cloud-fan
pushed a commit
that referenced
this pull request
Jul 13, 2021
…r change of trimming characters for cast
### What changes were proposed in this pull request?
This PR modifies comment for `UTF8String.trimAll` and`sql-migration-guide.mld`.
The comment for `UTF8String.trimAll` says like as follows.
```
Trims whitespaces ({literal <=} ASCII 32) from both ends of this string.
```
Similarly, `sql-migration-guide.md` mentions about the behavior of `cast` like as follows.
```
In Spark 3.0, when casting string value to integral types(tinyint, smallint, int and bigint),
datetime types(date, timestamp and interval) and boolean type,
the leading and trailing whitespaces (<= ASCII 32) will be trimmed before converted to these type values,
for example, `cast(' 1\t' as int)` results `1`, `cast(' 1\t' as boolean)` results `true`,
`cast('2019-10-10\t as date)` results the date value `2019-10-10`.
In Spark version 2.4 and below, when casting string to integrals and booleans,
it does not trim the whitespaces from both ends; the foregoing results is `null`,
while to datetimes, only the trailing spaces (= ASCII 32) are removed.
```
But SPARK-32559 (#29375) changed the behavior and only whitespace ASCII characters will be trimmed since Spark 3.0.1.
### Why are the changes needed?
To follow the previous change.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Confirmed the document built by the following command.
```
SKIP_API=1 bundle exec jekyll build
```
Closes #33287 from sarutak/fix-utf8string-trim-issue.
Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 57a4f31)
Signed-off-by: Wenchen Fan <[email protected]>
cloud-fan
pushed a commit
that referenced
this pull request
Jul 13, 2021
…r change of trimming characters for cast
### What changes were proposed in this pull request?
This PR modifies comment for `UTF8String.trimAll` and`sql-migration-guide.mld`.
The comment for `UTF8String.trimAll` says like as follows.
```
Trims whitespaces ({literal <=} ASCII 32) from both ends of this string.
```
Similarly, `sql-migration-guide.md` mentions about the behavior of `cast` like as follows.
```
In Spark 3.0, when casting string value to integral types(tinyint, smallint, int and bigint),
datetime types(date, timestamp and interval) and boolean type,
the leading and trailing whitespaces (<= ASCII 32) will be trimmed before converted to these type values,
for example, `cast(' 1\t' as int)` results `1`, `cast(' 1\t' as boolean)` results `true`,
`cast('2019-10-10\t as date)` results the date value `2019-10-10`.
In Spark version 2.4 and below, when casting string to integrals and booleans,
it does not trim the whitespaces from both ends; the foregoing results is `null`,
while to datetimes, only the trailing spaces (= ASCII 32) are removed.
```
But SPARK-32559 (#29375) changed the behavior and only whitespace ASCII characters will be trimmed since Spark 3.0.1.
### Why are the changes needed?
To follow the previous change.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Confirmed the document built by the following command.
```
SKIP_API=1 bundle exec jekyll build
```
Closes #33287 from sarutak/fix-utf8string-trim-issue.
Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 57a4f31)
Signed-off-by: Wenchen Fan <[email protected]>
flyrain
pushed a commit
to flyrain/spark
that referenced
this pull request
Sep 21, 2021
…r change of trimming characters for cast
### What changes were proposed in this pull request?
This PR modifies comment for `UTF8String.trimAll` and`sql-migration-guide.mld`.
The comment for `UTF8String.trimAll` says like as follows.
```
Trims whitespaces ({literal <=} ASCII 32) from both ends of this string.
```
Similarly, `sql-migration-guide.md` mentions about the behavior of `cast` like as follows.
```
In Spark 3.0, when casting string value to integral types(tinyint, smallint, int and bigint),
datetime types(date, timestamp and interval) and boolean type,
the leading and trailing whitespaces (<= ASCII 32) will be trimmed before converted to these type values,
for example, `cast(' 1\t' as int)` results `1`, `cast(' 1\t' as boolean)` results `true`,
`cast('2019-10-10\t as date)` results the date value `2019-10-10`.
In Spark version 2.4 and below, when casting string to integrals and booleans,
it does not trim the whitespaces from both ends; the foregoing results is `null`,
while to datetimes, only the trailing spaces (= ASCII 32) are removed.
```
But SPARK-32559 (apache#29375) changed the behavior and only whitespace ASCII characters will be trimmed since Spark 3.0.1.
### Why are the changes needed?
To follow the previous change.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Confirmed the document built by the following command.
```
SKIP_API=1 bundle exec jekyll build
```
Closes apache#33287 from sarutak/fix-utf8string-trim-issue.
Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 57a4f31)
Signed-off-by: Wenchen Fan <[email protected]>
yaooqinn
pushed a commit
that referenced
this pull request
Jun 13, 2023
…acters correctly ### What changes were proposed in this pull request? The trim logic in Cast expression introduced in #29375 trim ASCII control characters unexpectly. Before this patch  And hive  ### Why are the changes needed? The behavior described above doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? add ut Closes #41535 from Kwafoor/trim_bugfix. Lead-authored-by: wangjunbo <[email protected]> Co-authored-by: Junbo wang <[email protected]> Signed-off-by: Kent Yao <[email protected]>
yaooqinn
pushed a commit
that referenced
this pull request
Jun 13, 2023
…acters correctly ### What changes were proposed in this pull request? The trim logic in Cast expression introduced in #29375 trim ASCII control characters unexpectly. Before this patch  And hive  ### Why are the changes needed? The behavior described above doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? add ut Closes #41535 from Kwafoor/trim_bugfix. Lead-authored-by: wangjunbo <[email protected]> Co-authored-by: Junbo wang <[email protected]> Signed-off-by: Kent Yao <[email protected]> (cherry picked from commit 80588e4) Signed-off-by: Kent Yao <[email protected]>
yaooqinn
pushed a commit
that referenced
this pull request
Jun 13, 2023
…acters correctly ### What changes were proposed in this pull request? The trim logic in Cast expression introduced in #29375 trim ASCII control characters unexpectly. Before this patch  And hive  ### Why are the changes needed? The behavior described above doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? add ut Closes #41535 from Kwafoor/trim_bugfix. Lead-authored-by: wangjunbo <[email protected]> Co-authored-by: Junbo wang <[email protected]> Signed-off-by: Kent Yao <[email protected]> (cherry picked from commit 80588e4) Signed-off-by: Kent Yao <[email protected]>
czxm
pushed a commit
to czxm/spark
that referenced
this pull request
Jun 19, 2023
…acters correctly ### What changes were proposed in this pull request? The trim logic in Cast expression introduced in apache#29375 trim ASCII control characters unexpectly. Before this patch  And hive  ### Why are the changes needed? The behavior described above doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? add ut Closes apache#41535 from Kwafoor/trim_bugfix. Lead-authored-by: wangjunbo <[email protected]> Co-authored-by: Junbo wang <[email protected]> Signed-off-by: Kent Yao <[email protected]>
snmvaughan
pushed a commit
to snmvaughan/spark
that referenced
this pull request
Jun 20, 2023
…acters correctly ### What changes were proposed in this pull request? The trim logic in Cast expression introduced in apache#29375 trim ASCII control characters unexpectly. Before this patch  And hive  ### Why are the changes needed? The behavior described above doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? add ut Closes apache#41535 from Kwafoor/trim_bugfix. Lead-authored-by: wangjunbo <[email protected]> Co-authored-by: Junbo wang <[email protected]> Signed-off-by: Kent Yao <[email protected]> (cherry picked from commit 80588e4) Signed-off-by: Kent Yao <[email protected]>
GladwinLee
pushed a commit
to lyft/spark
that referenced
this pull request
Oct 10, 2023
…acters correctly ### What changes were proposed in this pull request? The trim logic in Cast expression introduced in apache#29375 trim ASCII control characters unexpectly. Before this patch  And hive  ### Why are the changes needed? The behavior described above doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? add ut Closes apache#41535 from Kwafoor/trim_bugfix. Lead-authored-by: wangjunbo <[email protected]> Co-authored-by: Junbo wang <[email protected]> Signed-off-by: Kent Yao <[email protected]> (cherry picked from commit 80588e4) Signed-off-by: Kent Yao <[email protected]>
catalinii
pushed a commit
to lyft/spark
that referenced
this pull request
Oct 10, 2023
…acters correctly ### What changes were proposed in this pull request? The trim logic in Cast expression introduced in apache#29375 trim ASCII control characters unexpectly. Before this patch  And hive  ### Why are the changes needed? The behavior described above doesn't consistent with the behavior of Hive ### Does this PR introduce _any_ user-facing change? Yes ### How was this patch tested? add ut Closes apache#41535 from Kwafoor/trim_bugfix. Lead-authored-by: wangjunbo <[email protected]> Co-authored-by: Junbo wang <[email protected]> Signed-off-by: Kent Yao <[email protected]> (cherry picked from commit 80588e4) Signed-off-by: Kent Yao <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
The trim logic in Cast expression introduced in #26622 trim non-ASCII characters unexpectly.
Before this patch
After this patch
Why are the changes needed?
The behavior described above doesn't make sense, and also doesn't consistent with the behavior when cast a string to double/float, as well as doesn't consistent with the behavior of Hive
Does this PR introduce any user-facing change?
Yes
How was this patch tested?
Added more UT