You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/07/10 06:06:42 UTC
[GitHub] [spark] sarutak opened a new pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
sarutak opened a new pull request #33287:
URL: https://github.com/apache/spark/pull/33287
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This PR fixes an issue that `UTF8String.trimAll` doesn't comply with its specification.
The comment about `UTF8String.trimAll` says:
```
Note that, this method is the same as java's {@link String#trim}, and different from
{@link UTF8String#trim()} which remove only spaces(= ASCII 32) from both ends.
```
So, like `String.trim`, all the characters in the range of `0x00` - `0x20` should be trimmed.
But the current implementation only trims characters for which `Character.isWhitespace` returns `true` (including whitespaces greater than 0x20 like `U+3000`).
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Bug fix.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No. The specification is not changed.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
New tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877620154
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45390/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877605801
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878705166
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45457/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878684219
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45452/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r668304359
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -573,15 +573,16 @@ public UTF8String trim() {
*/
public UTF8String trimAll() {
int s = 0;
+ byte currentByte;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && 0 <= (currentByte = getByte(s)) && currentByte <= 0x20) s++;
Review comment:
Thank you for your suggestion but this issue seems to be regarded as a doc issue. So, I've decided not to change the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877711102
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45394/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877605795
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45389/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877698430
**[Test build #140882 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140882/testReport)** for PR 33287 at commit [`b017b34`](https://github.com/apache/spark/commit/b017b34c68dd4577cee265ebb3b1ba682af4eef4).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877600773
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45389/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877612350
**[Test build #140879 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140879/testReport)** for PR 33287 at commit [`2330826`](https://github.com/apache/spark/commit/2330826560ca6ab49f8216a6b9aa20fb1061234d).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877593867
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140876/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878645282
@cloud-fan I understand that trimming characters (<= ASCII 32) was not intended behavior.
I've update the migration guide and comment for `UTF8String.trimAll`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878705166
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45457/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877611979
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140878/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877592957
**[Test build #140876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140876/testReport)** for PR 33287 at commit [`5f62cde`](https://github.com/apache/spark/commit/5f62cde99177da000076d4101add9663323af792).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877699932
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140882/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878703161
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45457/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877707381
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667380567
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
If we comply with the comment for `trimAll`, `<= 0x20` one seems correct. Here is the implementation of `String.trim`.
https://github.com/openjdk/jdk/blob/da75f3c4ad5bdf25167a3ed80e51f567ab3dbd01/src/java.base/share/classes/java/lang/StringLatin1.java#L531-L542
https://github.com/openjdk/jdk/blob/da75f3c4ad5bdf25167a3ed80e51f567ab3dbd01/src/java.base/share/classes/java/lang/StringUTF16.java#L847-L860
Also, I noticed that originally, characters <= 0x20 were trimmed but #29375 changed the behavior.
That change seems to break the compatibility.
`sql-migration-guide.md` says 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.
```
In fact, select `cast('2019-10-10\b' as date);` returns `2019-10-10` in `Spark 3.0.0`.
But after `3.0.1`, the query returns `NULL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667380567
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
If we comply with the comment for `trimAll`, `<= 0x20` one seems correct. Here is the implementation of `String.trim`.
https://github.com/openjdk/jdk/blob/da75f3c4ad5bdf25167a3ed80e51f567ab3dbd01/src/java.base/share/classes/java/lang/StringLatin1.java#L531-L542
https://github.com/openjdk/jdk/blob/da75f3c4ad5bdf25167a3ed80e51f567ab3dbd01/src/java.base/share/classes/java/lang/StringUTF16.java#L847-L860
Also, I noticed that originally, characters <= 0x20 were trimmed but #29375 changed the behavior.
That change seems to break the compatibility.
`sql-migration-guide.md` says 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.
```
In fact, select `cast('2019-10-10\b' as date);` returns `2019-10-10` in `Spark 3.0.0`.
But after `3.0.1`, the query returns `NULL`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667335982
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
Is either behavior more 'correct'? I'm not sure what this is trying to match. It's possible `.isWhitespace` is a better behavior, in which case the docs should change. By default I'd not change behavior unless there's a reason it's not intended.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877605801
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45389/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877577512
**[Test build #140876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140876/testReport)** for PR 33287 at commit [`5f62cde`](https://github.com/apache/spark/commit/5f62cde99177da000076d4101add9663323af792).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877707381
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45393/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877612350
**[Test build #140879 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140879/testReport)** for PR 33287 at commit [`2330826`](https://github.com/apache/spark/commit/2330826560ca6ab49f8216a6b9aa20fb1061234d).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878687971
**[Test build #140945 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140945/testReport)** for PR 33287 at commit [`168f3c8`](https://github.com/apache/spark/commit/168f3c8cce5a8a4bca4c4603f7a4dc3d7683c50b).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667380567
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
If we comply with the comment for `trimAll`, `<= 0x20` one seems correct. Here is the implementation of `String.trim`.
https://github.com/openjdk/jdk/blob/da75f3c4ad5bdf25167a3ed80e51f567ab3dbd01/src/java.base/share/classes/java/lang/StringLatin1.java#L531-L542
https://github.com/openjdk/jdk/blob/da75f3c4ad5bdf25167a3ed80e51f567ab3dbd01/src/java.base/share/classes/java/lang/StringUTF16.java#L847-L860
Also, I noticed that originally, characters <= 0x20 were trimmed but #29375 changed the behavior.
That change seems to break the compatibility.
`sql-migration-guide.md` says 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.
```
In fact, select `cast('2019-10-10\b' as date);` returns `2019-10-10` in `Spark 3.0.0`.
But after `3.0.1`, the query returns `NULL`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877720506
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877587202
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45387/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877593867
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140876/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667513166
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
cc: @cloud-fan and @yaooqinn who were involved in #29375 and #26622.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878686812
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140940/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877627893
**[Test build #140879 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140879/testReport)** for PR 33287 at commit [`2330826`](https://github.com/apache/spark/commit/2330826560ca6ab49f8216a6b9aa20fb1061234d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878687971
**[Test build #140945 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140945/testReport)** for PR 33287 at commit [`168f3c8`](https://github.com/apache/spark/commit/168f3c8cce5a8a4bca4c4603f7a4dc3d7683c50b).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878660400
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45452/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877587206
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45387/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878743114
**[Test build #140945 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140945/testReport)** for PR 33287 at commit [`168f3c8`](https://github.com/apache/spark/commit/168f3c8cce5a8a4bca4c4603f7a4dc3d7683c50b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878748106
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140945/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878641022
**[Test build #140940 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140940/testReport)** for PR 33287 at commit [`168f3c8`](https://github.com/apache/spark/commit/168f3c8cce5a8a4bca4c4603f7a4dc3d7683c50b).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877707381
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45393/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877699932
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140882/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878672206
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45452/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877620154
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45390/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667512892
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
> Looking at #29375 , it seems like the change was at least partly on purpose to catch 'whitespace' that isn't ASCII 32 or less
I think, the purpose of that change was to handle code points which is >= 0x80 (non-ASCII).
For example, `あ` is `00 81 82` in hex in `UTF-8`.
`getByte` returns `-127` for `0x81` so only checking `<= 0x20` is not enough.
I think this is the problem #29375 originally aimed to resolve.
But it should have checked whether a `byte` data is in the range of `0` and `0x20` to avoid breaking the compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878686044
**[Test build #140940 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140940/testReport)** for PR 33287 at commit [`168f3c8`](https://github.com/apache/spark/commit/168f3c8cce5a8a4bca4c4603f7a4dc3d7683c50b).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878704394
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45458/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877577512
**[Test build #140876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140876/testReport)** for PR 33287 at commit [`5f62cde`](https://github.com/apache/spark/commit/5f62cde99177da000076d4101add9663323af792).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878717230
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45458/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878727804
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45458/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878335670
`\b` means backspace, which is a control character that moves the cursor one character back in the console but doesn't delete it. I don't think we should trim it as whitespace.
I think this is a doc issue. The intention is to trim white spaces, but the condition (<= ASCII 32) was wrong and https://github.com/apache/spark/pull/29375 fixed it. Shall we fix the doc/comment instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878641022
**[Test build #140940 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140940/testReport)** for PR 33287 at commit [`168f3c8`](https://github.com/apache/spark/commit/168f3c8cce5a8a4bca4c4603f7a4dc3d7683c50b).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-879046436
thanks, merging to master/3.2/3.1/3.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877617581
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45390/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877698430
**[Test build #140882 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140882/testReport)** for PR 33287 at commit [`b017b34`](https://github.com/apache/spark/commit/b017b34c68dd4577cee265ebb3b1ba682af4eef4).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877709275
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45394/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877705842
**[Test build #140883 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140883/testReport)** for PR 33287 at commit [`f24e6a4`](https://github.com/apache/spark/commit/f24e6a41c5468bd7ed3fa5220dbbc1897df7a792).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877594225
**[Test build #140878 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140878/testReport)** for PR 33287 at commit [`19dfc60`](https://github.com/apache/spark/commit/19dfc6030fffc4e31259839989283cddaa65fbe9).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xkrogen commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
xkrogen commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r668048393
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -573,15 +573,16 @@ public UTF8String trim() {
*/
public UTF8String trimAll() {
int s = 0;
+ byte currentByte;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && 0 <= (currentByte = getByte(s)) && currentByte <= 0x20) s++;
Review comment:
Seems like this would be a good time to pull this logic out into a shared method? The if-condition is getting a bit more complicated and hard to read with the temporary variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877587206
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45387/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877628437
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140879/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878748106
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140945/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877707375
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45393/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #33287:
URL: https://github.com/apache/spark/pull/33287
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877628437
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140879/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877720506
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878263119
I don't know if I have the full context to evaluate this, but it seems reasonable to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877707381
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878727804
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45458/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877705842
**[Test build #140883 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140883/testReport)** for PR 33287 at commit [`f24e6a4`](https://github.com/apache/spark/commit/f24e6a41c5468bd7ed3fa5220dbbc1897df7a792).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877703422
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45393/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877594225
**[Test build #140878 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140878/testReport)** for PR 33287 at commit [`19dfc60`](https://github.com/apache/spark/commit/19dfc6030fffc4e31259839989283cddaa65fbe9).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877610080
**[Test build #140878 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140878/testReport)** for PR 33287 at commit [`19dfc60`](https://github.com/apache/spark/commit/19dfc6030fffc4e31259839989283cddaa65fbe9).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877705842
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877717656
**[Test build #140883 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140883/testReport)** for PR 33287 at commit [`f24e6a4`](https://github.com/apache/spark/commit/f24e6a41c5468bd7ed3fa5220dbbc1897df7a792).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36066][CORE] UTF8String.trimAll doesn't comply with its specification
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877582567
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45387/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667512892
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
> Looking at #29375 , it seems like the change was at least partly on purpose to catch 'whitespace' that isn't ASCII 32 or less
I think, the purpose of that change was to handle code points which is >= 0x80 (non-ASCII).
For example, `あ` is `00 81 82` in hex in `UTF-8`.
Before #29375, `getByte` returns `-127` for `0x81` so only checking `<= 0x20` is not enough.
I think this is the problem #29375 originally aimed to resolve.
But it should have checked whether a `byte` data is in the range of `0` and `0x20` to avoid breaking the compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878686917
retest this please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878684219
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45452/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877711102
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/45394/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Update the document about the behavior change of trimming characters for cast
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-878686812
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140940/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #33287:
URL: https://github.com/apache/spark/pull/33287#issuecomment-877699910
**[Test build #140882 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/140882/testReport)** for PR 33287 at commit [`b017b34`](https://github.com/apache/spark/commit/b017b34c68dd4577cee265ebb3b1ba682af4eef4).
* This patch **fails Java style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #33287: [SPARK-36081][SPARK-36066][SQL] Fix the compatibility breaking issue related to cast and UTF8String
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #33287:
URL: https://github.com/apache/spark/pull/33287#discussion_r667508922
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
##########
@@ -574,14 +574,14 @@ public UTF8String trim() {
public UTF8String trimAll() {
int s = 0;
// skip all of the whitespaces (<=0x20) in the left side
- while (s < this.numBytes && Character.isWhitespace(getByte(s))) s++;
+ while (s < this.numBytes && getByte(s) <= 0x20) s++;
Review comment:
Looking at https://github.com/apache/spark/pull/29375 , it seems like the change was at least partly on purpose to catch 'whitespace' that isn't ASCII 32 or less. @WangGuangxin is this change of behavior necessary? Do we need to check for `.isWhitespace` _or_ `<= 0x20`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org