Skip to content

Commit

Permalink
[SPARK-27346][SQL] Loosen the newline assert condition on 'examples' …
Browse files Browse the repository at this point in the history
…field in ExpressionInfo

## What changes were proposed in this pull request?

I haven't tested by myself on Windows and I am not 100% sure if this is going to cause an actual problem.

However, this one line:

https://github.com/apache/spark/blob/827383a97c11a61661440ff86ce0c3382a2a23b2/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionInfo.java#L82

made me to investigate a lot today.

Given my speculation, if Spark is built in Linux and it's executed on Windows, it looks possible for multiline strings, like,

https://github.com/apache/spark/blob/5264164a67df498b73facae207eda12ee133be7d/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala#L146-L150

to throw an exception because the newline in the binary is `\n` but `System.lineSeparator` returns `\r\n`.

I think this is not yet found because this particular codes are not released yet (see SPARK-26426).

Looks just better to loosen the condition and forget about this stuff.

This should be backported into branch-2.4 as well.

## How was this patch tested?

N/A

Closes #24274 from HyukjinKwon/SPARK-27346.

Authored-by: Hyukjin Kwon <gurwls223@apache.org>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
  • Loading branch information
HyukjinKwon committed Apr 2, 2019
1 parent 13c5c1f commit 949d712
Showing 1 changed file with 1 addition and 1 deletion.
Expand Up @@ -79,7 +79,7 @@ public ExpressionInfo(
assert name != null;
assert arguments != null;
assert examples != null;
assert examples.isEmpty() || examples.startsWith(System.lineSeparator() + " Examples:");
assert examples.isEmpty() || examples.contains(" Examples:");
assert note != null;
assert since != null;

Expand Down

0 comments on commit 949d712

Please sign in to comment.