You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by MaxGekk <gi...@git.apache.org> on 2018/03/11 17:32:08 UTC
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Prevent crashes on schema infe...
GitHub user MaxGekk opened a pull request:
https://github.com/apache/spark/pull/20796
[SPARK-23649][SQL] Prevent crashes on schema inferring of CSV containing wrong UTF-8 chars
## What changes were proposed in this pull request?
The mapping of UTF-8 char's first byte to char's size doesn't cover whole range 0-255. It is defined only for 0-253:
https://github.com/apache/spark/blob/master/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java#L60-L65
https://github.com/apache/spark/blob/master/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java#L190
If the first byte of a char is 253-255, IndexOutOfBoundsException is thrown. Besides of that values for 244-252 are not correct according to recent unicode standard for UTF-8: http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
As a consequence of the exception above, the length of input string in UTF-8 encoding cannot be calculated if the string contains chars started from 253 code. It is visible on user's side as for example crashing of schema inferring of csv file which contains such chars but the file can be read if the schema is specified explicitly or if the mode set to multiline.
The proposed changes build correct mapping of first byte of UTF-8 char to its size (now it covers all cases) and skip disallowed chars (counts it as one octet).
## How was this patch tested?
Added a test and a file with a char which is disallowed in UTF-8 - 0xFF.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MaxGekk/spark-1 skip-wrong-utf8-chars
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20796.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20796
----
commit 6d6b3ca8eedc7bf6381cb8d746fe8a3ee0a281b2
Author: Maxim Gekk <ma...@...>
Date: 2018-02-27T17:41:01Z
Test: skip bytes disallowed in UTF-8
commit 0f474a033f152340519b10676026b80b7593c2f5
Author: Maxim Gekk <ma...@...>
Date: 2018-02-27T17:43:53Z
Making correct map of first byte to char size in UTF-8 and skip bytes disallowed in UTF-8
commit 2ee661618a40544a8cbf3ac0794a1a6b86b6b62d
Author: Maxim Gekk <ma...@...>
Date: 2018-02-28T13:26:21Z
Check inferred schema and returned bad string
commit d6c5f02ea1a08513a54ea9f3b30986dd92188b3e
Author: Maxim Gekk <ma...@...>
Date: 2018-02-28T14:04:22Z
The test csv was simplified
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20796
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20796
test this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88394/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88340 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88340/testReport)** for PR 20796 at commit [`8a501e3`](https://github.com/apache/spark/commit/8a501e359eab47f08c44dcdf3d88a0dec3f71470).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on the issue:
https://github.com/apache/spark/pull/20796
@HyukjinKwon @maropu @cloud-fan @gatorsmile Please, review it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88174 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88174/testReport)** for PR 20796 at commit [`d6c5f02`](https://github.com/apache/spark/commit/d6c5f02ea1a08513a54ea9f3b30986dd92188b3e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Prevent crashes on schema infe...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175215287
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala ---
@@ -1279,4 +1280,57 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {
Row("0,2013-111-11 12:13:14") :: Row(null) :: Nil
)
}
+
+ def testHandlingUTF8Char(utf8char: Array[Byte]): Unit = {
+ val inHex = utf8char.map("%02x".format(_)).mkString("_")
+ test(s"SPARK-23649: handle the first byte of the char: $inHex") {
+ withTempPath { path =>
+ def getBytes(str: String): Array[Byte] = {
+ str.getBytes(StandardCharsets.UTF_8)
+ }
+ val filename = s"${path.getAbsolutePath}.csv"
+ val header = getBytes("code,channel\n")
+ val row = getBytes("ABGUN") ++ utf8char ++ getBytes(",United")
+ val content = header ++ row
+ Files.write(Paths.get(filename), content)
--- End diff --
we should specify encoding here, to avoid the case that different platform have different default values.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on the issue:
https://github.com/apache/spark/pull/20796
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175589638
--- Diff: common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java ---
@@ -791,4 +795,21 @@ public void trimRightWithTrimString() {
assertEquals(fromString("头"), fromString("头a???/").trimRight(fromString("数?/*&^%a")));
assertEquals(fromString("头"), fromString("头数b数数 [").trimRight(fromString(" []数b")));
}
+
+ @Test
+ public void skipWrongFirstByte() {
+ int[] wrongFirstBytes = {
--- End diff --
The bytes are not filtered by UTF8String methods. For instance, in the case of csv datasource the invalid bytes are just passed to the final result. See https://issues.apache.org/jira/browse/SPARK-23649
I have created a separate ticket to fix the issue: https://issues.apache.org/jira/browse/SPARK-23741 .
I am not sure that the issue of output of wrong UTF-8 chars should be addressed by this PR (this pr just fixes crashes on wrong input) because it could impact on users and other Spark components. Need to discuss it and test it carefully.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20796
the fix LGTM, we can add more tests for different ranges of the invalid chars.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20796
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175590297
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
--- End diff --
Here is the table of allowed first bytes:
<img width="540" alt="screen shot 2018-03-19 at 9 39 17 pm" src="https://user-images.githubusercontent.com/1580697/37623134-eca5756a-2bc3-11e8-99a0-3b538697d15c.png">
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88320/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175676498
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
--- End diff --
I added a comment about the first bytes disallowed by UTF-8. The comment describes from where the byte ranges and restrictions come otherwise the comments just duplicate the implementation.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88196 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88196/testReport)** for PR 20796 at commit [`d6c5f02`](https://github.com/apache/spark/commit/d6c5f02ea1a08513a54ea9f3b30986dd92188b3e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88340/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88313/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88320 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88320/testReport)** for PR 20796 at commit [`27e5a5b`](https://github.com/apache/spark/commit/27e5a5b3023fc8dd65b65c41650ae22dd531e395).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88316 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88316/testReport)** for PR 20796 at commit [`27e5a5b`](https://github.com/apache/spark/commit/27e5a5b3023fc8dd65b65c41650ae22dd531e395).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20796
thanks, merging to master/2.3/2.2!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88394 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88394/testReport)** for PR 20796 at commit [`5557a80`](https://github.com/apache/spark/commit/5557a80d4674e929332d9441342e5b90e314eb45).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88196/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/20796
Could you add more tests for all the invalid range `253-255` in `UTF8StringSuite`?
@HyukjinKwon Could you trigger this test? Also, kindly pinging cuz this is probably your domain?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20796
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20796
add to whitelist
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88384/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88384 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88384/testReport)** for PR 20796 at commit [`5557a80`](https://github.com/apache/spark/commit/5557a80d4674e929332d9441342e5b90e314eb45).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20796
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175542711
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
--- End diff --
yea, seems we should need to list `0xC0, 0xC1` here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175573949
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
--- End diff --
Actually this table is from the unicode standard (10.0, Table 3-6, page 126):
<img width="571" alt="screen shot 2018-03-19 at 9 23 14 pm" src="https://user-images.githubusercontent.com/1580697/37620318-2c173f7e-2bbc-11e8-8fa5-5f04d11925de.png">
0xC0, 0xC1 are first bytes of 2 bytes chars disallowed by UTF-8 (for now)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20796
LGTM, pending jenkins
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88313 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88313/testReport)** for PR 20796 at commit [`c65f827`](https://github.com/apache/spark/commit/c65f82782443c22cfcdc86dd01195fe8f623f2a4).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175543480
--- Diff: common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java ---
@@ -791,4 +795,21 @@ public void trimRightWithTrimString() {
assertEquals(fromString("头"), fromString("头a???/").trimRight(fromString("数?/*&^%a")));
assertEquals(fromString("头"), fromString("头数b数数 [").trimRight(fromString(" []数b")));
}
+
+ @Test
+ public void skipWrongFirstByte() {
+ int[] wrongFirstBytes = {
--- End diff --
what will happen if we print UTF8String with invalid bytes?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88316/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88384 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88384/testReport)** for PR 20796 at commit [`5557a80`](https://github.com/apache/spark/commit/5557a80d4674e929332d9441342e5b90e314eb45).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88413/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88316 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88316/testReport)** for PR 20796 at commit [`27e5a5b`](https://github.com/apache/spark/commit/27e5a5b3023fc8dd65b65c41650ae22dd531e395).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20796
LGTM for the fix. +1 for more tests.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175281945
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
--- End diff --
hmm, is this `0xC2..0xDF`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88394 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88394/testReport)** for PR 20796 at commit [`5557a80`](https://github.com/apache/spark/commit/5557a80d4674e929332d9441342e5b90e314eb45).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175626064
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -187,8 +218,9 @@ public void writeTo(OutputStream out) throws IOException {
* @param b The first byte of a code point
*/
private static int numBytesForFirstByte(final byte b) {
- final int offset = (b & 0xFF) - 192;
- return (offset >= 0) ? bytesOfCodePointInUTF8[offset] : 1;
+ final int offset = b & 0xFF;
+ byte numBytes = bytesOfCodePointInUTF8[offset];
+ return (numBytes == 0) ? 1: numBytes; // Skip the first byte disallowed in UTF-8
--- End diff --
Is the comment valid? Do we skip it? Don't we still count the disallowed byte as one code point in `numChars`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Prevent crashes on schema infe...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r174293523
--- Diff: sql/core/src/test/resources/test-data/utf8xFF.csv ---
@@ -0,0 +1,3 @@
+channel,code
+United,123
+ABGUN�,456
--- End diff --
how did you create this file?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88174 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88174/testReport)** for PR 20796 at commit [`d6c5f02`](https://github.com/apache/spark/commit/d6c5f02ea1a08513a54ea9f3b30986dd92188b3e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20796
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175577812
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
+ * 1110xxxx 0xE0..0xEF First byte of a 3-byte character encoding
+ * 11110xxx 0xF0..0xF4 First byte of a 4-byte character encoding
--- End diff --
I will add additional comment about which bytes are not allowed according to the table:
<img width="540" alt="screen shot 2018-03-19 at 9 39 17 pm" src="https://user-images.githubusercontent.com/1580697/37621033-21ffbae6-2bbe-11e8-9a8f-ec05263ef7f7.png">
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88196 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88196/testReport)** for PR 20796 at commit [`d6c5f02`](https://github.com/apache/spark/commit/d6c5f02ea1a08513a54ea9f3b30986dd92188b3e).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88320 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88320/testReport)** for PR 20796 at commit [`27e5a5b`](https://github.com/apache/spark/commit/27e5a5b3023fc8dd65b65c41650ae22dd531e395).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on the issue:
https://github.com/apache/spark/pull/20796
test this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175542911
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
+ * 1110xxxx 0xE0..0xEF First byte of a 3-byte character encoding
+ * 11110xxx 0xF0..0xF4 First byte of a 4-byte character encoding
--- End diff --
and also `0xF5..0xFF`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88174/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175624764
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -57,12 +57,39 @@
public Object getBaseObject() { return base; }
public long getBaseOffset() { return offset; }
- private static int[] bytesOfCodePointInUTF8 = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
- 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
- 4, 4, 4, 4, 4, 4, 4, 4,
- 5, 5, 5, 5,
- 6, 6};
+ /**
+ * A char in UTF-8 encoding can take 1-4 bytes depending on the first byte which
+ * indicates the size of the char. See Unicode standard in page 126:
+ * http://www.unicode.org/versions/Unicode10.0.0/UnicodeStandard-10.0.pdf
+ *
+ * Binary Hex Comments
+ * 0xxxxxxx 0x00..0x7F Only byte of a 1-byte character encoding
+ * 10xxxxxx 0x80..0xBF Continuation bytes (1-3 continuation bytes)
+ * 110xxxxx 0xC0..0xDF First byte of a 2-byte character encoding
--- End diff --
Yes, `0xC2..0xDF` should be the first byte for 2-byte character encoding. But here you said `0xC0..0xDF`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88313 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88313/testReport)** for PR 20796 at commit [`c65f827`](https://github.com/apache/spark/commit/c65f82782443c22cfcdc86dd01195fe8f623f2a4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Skipping chars disallowed in U...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r175679506
--- Diff: common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
@@ -187,8 +218,9 @@ public void writeTo(OutputStream out) throws IOException {
* @param b The first byte of a code point
*/
private static int numBytesForFirstByte(final byte b) {
- final int offset = (b & 0xFF) - 192;
- return (offset >= 0) ? bytesOfCodePointInUTF8[offset] : 1;
+ final int offset = b & 0xFF;
+ byte numBytes = bytesOfCodePointInUTF8[offset];
+ return (numBytes == 0) ? 1: numBytes; // Skip the first byte disallowed in UTF-8
--- End diff --
I think so. We jump over (skip by definition) such bytes and count it as one entity. If we don't count the bytes, we break `substring`, `toUpperCase`, `toLowerCase`, `trimRight/trimLeft` and etc. The reason of the changes is to not crash on bad input as previously we threw IndexOutOfBoundsexception on some wrong chars but could pass (count as 1) another wrong chars. This PR allows to cover whole range. I believe ignoring/removing of wrong chars should be addressed in changes for https://issues.apache.org/jira/browse/SPARK-23741
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88413 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88413/testReport)** for PR 20796 at commit [`5557a80`](https://github.com/apache/spark/commit/5557a80d4674e929332d9441342e5b90e314eb45).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20796: [SPARK-23649][SQL] Prevent crashes on schema infe...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20796#discussion_r174355311
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala ---
@@ -1279,4 +1279,22 @@ class CSVSuite extends QueryTest with SharedSQLContext with SQLTestUtils {
Row("0,2013-111-11 12:13:14") :: Row(null) :: Nil
)
}
+
+ test("skip the first byte of a char if it is disallowed in UTF-8") {
+ val df = spark.read
+ .format("csv")
+ .option("header", "true")
+ .load(testFile("test-data/utf8xFF.csv"))
+ val expectedSchema = new StructType()
+ .add("channel", StringType)
+ .add("code", StringType)
+
+ assert(df.schema == expectedSchema)
+
+ val badStr = new String("ABGUN".getBytes :+ 0xff.toByte)
--- End diff --
Shall we explicitly give encoding?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88413 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88413/testReport)** for PR 20796 at commit [`5557a80`](https://github.com/apache/spark/commit/5557a80d4674e929332d9441342e5b90e314eb45).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Prevent crashes on schema inferring o...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20796
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20796: [SPARK-23649][SQL] Skipping chars disallowed in UTF-8
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20796
**[Test build #88340 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88340/testReport)** for PR 20796 at commit [`8a501e3`](https://github.com/apache/spark/commit/8a501e359eab47f08c44dcdf3d88a0dec3f71470).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org