-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-28023][SQL] Add trim logic in UTF8String's toInt/toLong to make it consistent with other string-numeric casting #26622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
48d5a9f
05ab098
c769c40
ee94f98
4fa535b
2261433
cce5a55
4fd810a
cf2c243
98892fe
d0bade1
5b75d37
09f38f7
76639b7
f37f467
d5c2a40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1063,7 +1063,7 @@ public static class IntWrapper implements Serializable { | |
| } | ||
|
|
||
| /** | ||
| * Parses this UTF8String to long. | ||
| * Parses this UTF8String(trimmed if needed) to long. | ||
| * | ||
| * Note that, in this method we accumulate the result in negative format, and convert it to | ||
| * positive format at the end, if this string is not started with '-'. This is because min value | ||
|
|
@@ -1077,26 +1077,28 @@ public static class IntWrapper implements Serializable { | |
| * @return true if the parsing was successful else false | ||
| */ | ||
| public boolean toLong(LongWrapper toLongResult) { | ||
| if (numBytes == 0) { | ||
| return false; | ||
| } | ||
| int offset = 0; | ||
| while (offset < this.numBytes && getByte(offset) <= ' ') offset++; | ||
| if (offset == this.numBytes) return false; | ||
|
|
||
| byte b = getByte(0); | ||
| int end = this.numBytes - 1; | ||
| while (end > offset && getByte(end) <= ' ') end--; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need to trim from the right explicitly here. Just break inside the loop https://github.com/apache/spark/pull/26622/files#diff-d2b5337b91f684b9e7fd5cc101e93fc8R1104 if b == ' '
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I guess not, how do you know the ' ' is in the middle or end?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see |
||
|
|
||
| byte b = getByte(offset); | ||
| final boolean negative = b == '-'; | ||
| int offset = 0; | ||
| if (negative || b == '+') { | ||
| offset++; | ||
| if (numBytes == 1) { | ||
| if (end - offset == 0) { | ||
| return false; | ||
| } | ||
| offset++; | ||
| } | ||
|
|
||
| final byte separator = '.'; | ||
| final int radix = 10; | ||
| final long stopValue = Long.MIN_VALUE / radix; | ||
| long result = 0; | ||
|
|
||
| while (offset < numBytes) { | ||
| while (offset <= end) { | ||
| b = getByte(offset); | ||
| offset++; | ||
| if (b == separator) { | ||
|
|
@@ -1131,7 +1133,7 @@ public boolean toLong(LongWrapper toLongResult) { | |
| // This is the case when we've encountered a decimal separator. The fractional | ||
| // part will not change the number, but we will verify that the fractional part | ||
| // is well formed. | ||
| while (offset < numBytes) { | ||
| while (offset <= end) { | ||
| byte currentByte = getByte(offset); | ||
| if (currentByte < '0' || currentByte > '9') { | ||
| return false; | ||
|
|
@@ -1151,7 +1153,7 @@ public boolean toLong(LongWrapper toLongResult) { | |
| } | ||
|
|
||
| /** | ||
| * Parses this UTF8String to int. | ||
| * Parses this UTF8String(trimmed if needed) to int. | ||
| * | ||
| * Note that, in this method we accumulate the result in negative format, and convert it to | ||
| * positive format at the end, if this string is not started with '-'. This is because min value | ||
|
|
@@ -1168,26 +1170,28 @@ public boolean toLong(LongWrapper toLongResult) { | |
| * @return true if the parsing was successful else false | ||
| */ | ||
| public boolean toInt(IntWrapper intWrapper) { | ||
| if (numBytes == 0) { | ||
| return false; | ||
| } | ||
| int offset = 0; | ||
| while (offset < this.numBytes && getByte(offset) <= ' ') offset++; | ||
| if (offset == this.numBytes) return false; | ||
|
|
||
| byte b = getByte(0); | ||
| int end = this.numBytes - 1; | ||
| while (end > offset && getByte(end) <= ' ') end--; | ||
|
|
||
| byte b = getByte(offset); | ||
| final boolean negative = b == '-'; | ||
| int offset = 0; | ||
| if (negative || b == '+') { | ||
| offset++; | ||
| if (numBytes == 1) { | ||
| if (end - offset == 0) { | ||
| return false; | ||
| } | ||
| offset++; | ||
| } | ||
|
|
||
| final byte separator = '.'; | ||
| final int radix = 10; | ||
| final int stopValue = Integer.MIN_VALUE / radix; | ||
| int result = 0; | ||
|
|
||
| while (offset < numBytes) { | ||
| while (offset <= end) { | ||
| b = getByte(offset); | ||
| offset++; | ||
| if (b == separator) { | ||
|
|
@@ -1222,7 +1226,7 @@ public boolean toInt(IntWrapper intWrapper) { | |
| // This is the case when we've encountered a decimal separator. The fractional | ||
| // part will not change the number, but we will verify that the fractional part | ||
| // is well formed. | ||
| while (offset < numBytes) { | ||
| while (offset <= end) { | ||
| byte currentByte = getByte(offset); | ||
| if (currentByte < '0' || currentByte > '9') { | ||
| return false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,13 @@ | ||
| -- binary type | ||
| select x'00' < x'0f'; | ||
| select x'00' < x'ff'; | ||
|
|
||
| -- trim string to numeric | ||
| select '1 ' = 1Y; | ||
| select '\t1 ' = 1Y; | ||
| select '1 ' = 1S; | ||
| select '1 ' = 1; | ||
cloud-fan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| select ' 1' = 1L; | ||
| select ' 1' = cast(1.0 as float); | ||
| select ' 1.0 ' = 1.0D; | ||
| select ' 1.0 ' = 1.0BD; | ||
Uh oh!
There was an error while loading. Please reload this page.