You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2024/10/25 07:01:35 UTC
[PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
panbingkun opened a new pull request, #48653:
URL: https://github.com/apache/spark/pull/48653
### What changes were proposed in this pull request?
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
Yes,
### How was this patch tested?
Pass GA.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1837539335
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +185,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(retryWithTrim[Byte](_.toByte))
Review Comment:
the performance will be terrible if most data has whitespace, as now we pay the per-row cost of error handling.
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1816969618
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +179,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(_.trim.toByte)
Review Comment:
I changed the way of writing, after `raising an exception` `NumberFormatException` or `IllegalArgumentException`, re-parse after trim it.
I will update results of `CSV benchmark`.
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2468210110
Let's make it clear in the PR title and description. Are you suggesting to turn on `gnoreLeadingWhiteSpace` by default?
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1837600750
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +185,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(retryWithTrim[Byte](_.toByte))
Review Comment:
I even suspect it's a problem with JDK itself, haha
A.Int: java.lang.Integer.parseInt(...)
```
scala> val x = java.lang.Integer.parseInt(" 1")
java.lang.NumberFormatException: For input string: " 1"
at java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:67)
at java.base/java.lang.Integer.parseInt(Integer.java:654)
at java.base/java.lang.Integer.parseInt(Integer.java:786)
... 41 elided
```
B.Double: java.lang.Double.parseDouble(...)
```scala
scala> val x = java.lang.Double.parseDouble(" 1")
val x: Double = 1.0
```
```java
public static double parseDouble(String s) throws NumberFormatException {
return FloatingDecimal.parseDouble(s);
}
```
<img width="800" alt="image" src="https://github.com/user-attachments/assets/406ca81f-2d11-4a1a-9f42-bd8e5ffab03f">
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1816974141
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +179,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(_.trim.toByte)
Review Comment:
The result of `CSVBenchmark`:
JDK 17: https://github.com/panbingkun/spark/actions/runs/11521536124
JDK 21: https://github.com/panbingkun/spark/actions/runs/11521541767
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2437104578
Although we can set `ignoreLeadingWhiteSpace` to `true` to display it correctly, the type `float` & `double` can still be displayed correctly `without` setting it,
```sql
spark-sql (default)> select from_csv('1, 1', 'a INT, b INT', map('ignoreLeadingWhiteSpace', 'false'));
from_csv(1, 1)
{"a":1,"b":null}
Time taken: 0.099 seconds, Fetched 1 row(s)
spark-sql (default)> select from_csv('1, 1', 'a INT, b INT', map('ignoreLeadingWhiteSpace', 'true'));
from_csv(1, 1)
{"a":1,"b":1}
Time taken: 0.037 seconds, Fetched 1 row(s)
```
```
spark-sql (default)> select from_csv('1, 1', 'a INT, b DOUBLE', map('ignoreLeadingWhiteSpace', 'false'));
from_csv(1, 1)
{"a":1,"b":1.0}
Time taken: 0.065 seconds, Fetched 1 row(s)
spark-sql (default)> select from_csv('1, 1', 'a INT, b DOUBLE', map('ignoreLeadingWhiteSpace', 'true'));
from_csv(1, 1)
{"a":1,"b":1.0}
Time taken: 0.035 seconds, Fetched 1 row(s)
```
and in most cases, users think it is normal to use `", "` (`Please note that there is a space after the comma`) to separate fields in `CSV`, as in our example:
https://github.com/apache/spark/blob/6e82d5946ae73bd0f7d0f9b1826a0e4a29eaaaec/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/csvExpressions.scala#L46-L49
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1816963974
##########
sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala:
##########
@@ -485,7 +486,7 @@ class CsvFunctionsSuite extends QueryTest with SharedSparkSession {
test("SPARK-32968: bad csv input with csv pruning optimization") {
Seq("true", "false").foreach { enabled =>
withSQLConf(SQLConf.CSV_EXPRESSION_OPTIMIZATION.key -> enabled) {
- val df = sparkContext.parallelize(Seq("1,\u0001\u0000\u0001234")).toDF("csv")
+ val df = sparkContext.parallelize(Seq("1,\u0001\u0000\u0001234X")).toDF("csv")
Review Comment:
I think the purpose of case `SPARK-32968: bad csv input with csv pruning optimization` is to test for illegal characters causing parse `exceptions`. After this PR, it should be able to parse `correctly` (from `exception` to `normal`, it should not belong to `break changes`)
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1837586746
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +185,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(retryWithTrim[Byte](_.toByte))
Review Comment:
Maybe this isn't an issue, as people can turn on `ignoreLeadingWhiteSpace`.
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2437106015
cc @MaxGekk @HyukjinKwon @cloud-fan
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1837581169
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +185,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(retryWithTrim[Byte](_.toByte))
Review Comment:
What do you think of the first version of this PR?
<img width="1037" alt="image" src="https://github.com/user-attachments/assets/564e4146-d0b5-4d6f-99a7-924a6ea042f1">
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2673022549
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1816618015
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +179,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(_.trim.toByte)
Review Comment:
All the trims might lead to perf regressions. Need to update results of CSV benchmark, I guess.
##########
sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala:
##########
@@ -485,7 +486,7 @@ class CsvFunctionsSuite extends QueryTest with SharedSparkSession {
test("SPARK-32968: bad csv input with csv pruning optimization") {
Seq("true", "false").foreach { enabled =>
withSQLConf(SQLConf.CSV_EXPRESSION_OPTIMIZATION.key -> enabled) {
- val df = sparkContext.parallelize(Seq("1,\u0001\u0000\u0001234")).toDF("csv")
+ val df = sparkContext.parallelize(Seq("1,\u0001\u0000\u0001234X")).toDF("csv")
Review Comment:
why do you need to change this?
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2467641107
> Is this behavior consistent with reading CSV files?
- test.csv
```csv
1, 1
```
- Before this PR
A.schema = "a INT, b DOUBLE"
```scala
test("read csv test.csv") {
val df = spark.read.schema("a INT, b DOUBLE").csv(testFile("test-data/test.csv"))
df.show(true)
}
```
result
```
+---+---+
| a| b|
+---+---+
| 1|1.0|
+---+---+
```
B.schema = "a INT, b INT"
```scala
test("read csv test.csv") {
val df = spark.read.schema("a INT, b INT").csv(testFile("test-data/test.csv"))
df.show(true)
}
```
result
```
+---+----+
| a| b|
+---+----+
| 1|NULL|
+---+----+
```
- After this PR
A.schema = "a INT, b DOUBLE"
```scala
test("read csv test.csv") {
val df = spark.read.schema("a INT, b DOUBLE").csv(testFile("test-data/test.csv"))
df.show(true)
}
```
result:
```
+---+---+
| a| b|
+---+---+
| 1|1.0|
+---+---+
```
B.schema = "a INT, b INT"
```scala
test("read csv test.csv") {
val df = spark.read.schema("a INT, b INT").csv(testFile("test-data/test.csv"))
df.show(true)
}
```
result:
```
+---+---+
| a| b|
+---+---+
| 1| 1|
+---+---+
```
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2468358508
> Let's make it clear in the PR title and description.
Updated.
> Are you suggesting to turn on `ignoreLeadingWhiteSpace` by default?
I think this is indeed a solution that can solve similar scenarios, but will it affect the reading performance of data without spaces? I am not sure because I have preliminarily looked at the `CsvParser` logic and it is as follows:
<img width="714" alt="image" src="https://github.com/user-attachments/assets/0eae4f28-b19c-45c4-af53-689c407f1947">
<img width="954" alt="image" src="https://github.com/user-attachments/assets/f939daf8-ae8c-4aab-946e-a5760ce0e3a9">
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2465244299
@panbingkun There are CSV options:
- ignoreLeadingWhiteSpace
- ignoreTrailingWhiteSpace
They are off by default, but when you set them on, do they solve your issue?
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2467385609
Is this behavior consistent with reading CSV files?
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2467673599
So this PR is trying to update both `from_csv` and reading CSV files?
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2465929498
> @panbingkun There are CSV options:
>
> * ignoreLeadingWhiteSpace
> * ignoreTrailingWhiteSpace
>
> They are off in read by default, but when you set them on, do they solve your issue?
- Taking the following as an example:
```sql
# parse error
spark-sql (default)> select from_csv('1, 1', 'a INT, b INT', map('ignoreLeadingWhiteSpace', 'false'));
from_csv(1, 1)
{"a":1,"b":null}
Time taken: 0.099 seconds, Fetched 1 row(s)
# parse correctly
spark-sql (default)> select from_csv('1, 1', 'a INT, b INT', map('ignoreLeadingWhiteSpace', 'true'));
from_csv(1, 1)
{"a":1,"b":1}
Time taken: 0.037 seconds, Fetched 1 row(s)
```
```sql
# parse correctly
spark-sql (default)> select from_csv('1, 1', 'a INT, b DOUBLE', map('ignoreLeadingWhiteSpace', 'false'));
from_csv(1, 1)
{"a":1,"b":1.0}
Time taken: 0.065 seconds, Fetched 1 row(s)
# parse correctly
spark-sql (default)> select from_csv('1, 1', 'a INT, b DOUBLE', map('ignoreLeadingWhiteSpace', 'true'));
from_csv(1, 1)
{"a":1,"b":1.0}
Time taken: 0.035 seconds, Fetched 1 row(s)
```
- Yes, For the same data `1, 1`, when the schema is `a INT, b INT`, setting `gnoreLeadingWhiteSpace = true` can be bypassed to obtain the correct parsing. However, when the schema is 'a INT, b DOUBLE', there is no need to set it, and the correct parsing can be obtained, this behavior looks very `weird`.
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1816957991
##########
sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala:
##########
@@ -485,7 +486,7 @@ class CsvFunctionsSuite extends QueryTest with SharedSparkSession {
test("SPARK-32968: bad csv input with csv pruning optimization") {
Seq("true", "false").foreach { enabled =>
withSQLConf(SQLConf.CSV_EXPRESSION_OPTIMIZATION.key -> enabled) {
- val df = sparkContext.parallelize(Seq("1,\u0001\u0000\u0001234")).toDF("csv")
+ val df = sparkContext.parallelize(Seq("1,\u0001\u0000\u0001234X")).toDF("csv")
Review Comment:
Because:
<img width="918" alt="image" src="https://github.com/user-attachments/assets/089831c5-0d53-403c-9b08-fec433869229">
<img width="582" alt="image" src="https://github.com/user-attachments/assets/09235ef7-cd96-48c3-ab85-49c47e860381">
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1817443745
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +179,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(_.trim.toByte)
Review Comment:
I have updated results of `CSV benchmark`.
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #48653: [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after
URL: https://github.com/apache/spark/pull/48653
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1837582190
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +185,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(retryWithTrim[Byte](_.toByte))
Review Comment:
Currently, I haven't come up with a better solution.
--
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
Re: [PR] [SPARK-50110][SQL] Fix read csv fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #48653:
URL: https://github.com/apache/spark/pull/48653#discussion_r1837597575
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/csv/UnivocityParser.scala:
##########
@@ -179,16 +185,16 @@ class UnivocityParser(
dataType: DataType,
nullable: Boolean = true): ValueConverter = dataType match {
case _: ByteType => (d: String) =>
- nullSafeDatum(d, name, nullable, options)(_.toByte)
+ nullSafeDatum(d, name, nullable, options)(retryWithTrim[Byte](_.toByte))
Review Comment:
Okay, when the end-user sees a return value of `NULL`, we don't seem to have `a better way` to notice them (if an exception occurs, we may also notice them in the error message, but in our scenario, it won't). Alright, let's leave it for now.
Thanks!
--
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
Re: [PR] [SPARK-50110][SQL] Fix `from_csv`: parse fails when data contains spaces before and after [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #48653:
URL: https://github.com/apache/spark/pull/48653#issuecomment-2467866236
> So this PR is trying to update both `from_csv` and reading CSV files?
Yes, the underlying logic for `reading CSV files` and `from_csv` uses `UnivocityParser`
- v1
https://github.com/apache/spark/blob/a08859760a41bde9642307483c5d282c82d31bff/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala#L58
- v2
https://github.com/apache/spark/blob/a08859760a41bde9642307483c5d282c82d31bff/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/csv/CSVPartitionReaderFactory.scala#L56
- CsvToStructs(`from_csv`)
https://github.com/apache/spark/blob/a08859760a41bde9642307483c5d282c82d31bff/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/csvExpressions.scala#L129
--
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