-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-40194][SQL] SPLIT function on empty regex should truncate trailing empty string. #37631
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 3 commits
2e5d698
885106c
7a67a41
34c6b47
ff17ee4
29344fc
74b3f0b
e05fd28
7f6c0dd
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 |
|---|---|---|
|
|
@@ -1000,6 +1000,17 @@ public static UTF8String concatWs(UTF8String separator, UTF8String... inputs) { | |
| } | ||
|
|
||
| public UTF8String[] split(UTF8String pattern, int limit) { | ||
| // This is a special case for converting string into array of symbols without a trailing empty | ||
| // string. E.g. `"hello".split("", 0) => ["h", "e", "l", "l", "o"]. | ||
| // Note that negative limit will preserve a trailing empty string. | ||
| if (limit == 0 && numBytes() != 0 && pattern.numBytes() == 0) { | ||
| byte[] input = getBytes(); | ||
| UTF8String[] result = new UTF8String[numBytes]; | ||
| for (int i = 0; i < numBytes; i++) { | ||
|
Contributor
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. are we sure it's one byte per character?
Contributor
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. Great catch, fixed! |
||
| result[i] = UTF8String.fromBytes(input, i, 1); | ||
| } | ||
| return result; | ||
| } | ||
| return split(pattern.toString(), limit); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -527,7 +527,7 @@ case class StringSplit(str: Expression, regex: Expression, limit: Expression) | |
| override def second: Expression = regex | ||
| override def third: Expression = limit | ||
|
|
||
| def this(exp: Expression, regex: Expression) = this(exp, regex, Literal(-1)); | ||
| def this(exp: Expression, regex: Expression) = this(exp, regex, Literal(0)) | ||
|
Contributor
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. ideally 0 and -1 should be no difference according to the function doc. Do you use -1 as a legacy flag?
Contributor
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. Yes, you are correct. This should not change. Reverted. |
||
|
|
||
| override def nullSafeEval(string: Any, regex: Any, limit: Any): Any = { | ||
| val strings = string.asInstanceOf[UTF8String].split( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to update this comment now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thank you for reviewing!