Should read null properly when schema is lager than parsed tokens in PrunedScan #420
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #420 +/- ##
==========================================
+ Coverage 87.01% 87.12% +0.11%
==========================================
Files 12 12
Lines 562 567 +5
Branches 136 144 +8
==========================================
+ Hits 489 494 +5
Misses 73 73
Continue to review full report at Codecov.
|
HyukjinKwon
commented
Mar 28, 2017
| // If the given column is not nullable, we simply fall back to normal string | ||
| // rather than returning null for backwards compatibility. Note that this case is | ||
| // different with Spark's internal CSV datasource. | ||
| val isNullValueMatched = datum == nullValue && nullable |
There was a problem hiding this comment.
I think this case is different with Spark currently but did not change the behaviour here.
In case of Spark, it throws an exception in this case rather than failing back to string as below:
val schema = StructType(
StructField("a", IntegerType, true) ::
StructField("b", IntegerType, false) :: Nil)
spark.read.schema(schema)
.option("header", "false")
.option("nullValue", "2")
.option("mode", "FAILFAST")
.csv(Seq("1,2").toDS())
.show()prints
java.lang.RuntimeException: null value found but field b is not nullable.
at org.apache.spark.sql.execution.datasources.csv.UnivocityParser.org$apache$spark$sql$execution$datasources$csv$UnivocityParser$$nullSafeDatum(UnivocityParser.scala:179)
at org.apache.spark.sql.execution.datasources.csv.UnivocityParser$$anonfun$makeConverter$3.apply(UnivocityParser.scala:104)
at org.apache.spark.sql.execution.datasources.csv.UnivocityParser$$anonfun$makeConverter$3.apply(UnivocityParser.scala:103)
at org.apache.spark.sql.execution.datasources.csv.UnivocityParser.org$apache$spark$sql$execution$datasources$csv$UnivocityParser$$convert(UnivocityParser.scala:219)
at org.apache.spark.sql.execution.datasources.csv.UnivocityParser.parse(UnivocityParser.scala:191)
In case of this library,
val schema = StructType(
StructField("a", IntegerType, true) ::
StructField("b", IntegerType, false) :: Nil)
new CsvParser()
.withSchema(schema)
.withUseHeader(false)
.withParseMode(ParseModes.FAIL_FAST_MODE)
.withNullValue("2")
.csvRdd(sqlContext, sqlContext.sparkContext.parallelize(Seq("1,2")))
.show()prints below:
+---+---+
| a| b|
+---+---+
| 1| 2|
+---+---+
There was a problem hiding this comment.
I intended to not break this behaviour in this library but I am willing to match this to Spark's internal CSV one if you think so.
HyukjinKwon
commented
Mar 28, 2017
| val exception3 = intercept[RuntimeException]{ | ||
| TypeCast.castTo(null, "_c", StringType, nullable = false) | ||
| } | ||
| assert(exception3.getMessage.contains("null value found but field _c is not nullable")) |
There was a problem hiding this comment.
Here are the actual test cases added.
HyukjinKwon
commented
Mar 28, 2017
|
|
||
| // `datum` can be null when some tokens were inserted when permissive modes via `PrunedScan`. | ||
| // In this case, they are treated as nulls. | ||
| val isNullDatum = datum == null |
There was a problem hiding this comment.
Here, the actual cases added are,
datum == null && !nullable: returnsnull. (it was NEP orjava.lang.NumberFormatException: nullbefore).datum == null && !nullable: throws an exception. (it was NEP orjava.lang.NumberFormatException: nullbefore).treatEmptyValuesAsNulls && datum == && !nullable: throws an exception. (it was trying to setnullto non-nullable field.
These should be non-behaviour changes.
|
(Two comments were folded by the last commit but still valid to explain the changes here) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently running the codes below:
with the data below:
throws an exception as below:
This is related with apache/spark#15767. Please refer the description there.