[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-11 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/22367


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-10 Thread MaxGekk
Github user MaxGekk commented on a diff in the pull request:

https://github.com/apache/spark/pull/22367#discussion_r216463293
  
--- Diff: docs/sql-programming-guide.md ---
@@ -1897,6 +1897,7 @@ working with timestamps in `pandas_udf`s to get the 
best performance, see
   - In version 2.3 and earlier, CSV rows are considered as malformed if at 
least one column value in the row is malformed. CSV parser dropped such rows in 
the DROPMALFORMED mode or outputs an error in the FAILFAST mode. Since Spark 
2.4, CSV row is considered as malformed only when it contains malformed column 
values requested from CSV datasource, other values can be ignored. As an 
example, CSV file contains the "id,name" header and one row "1234". In Spark 
2.4, selection of the id column consists of a row with one column value 1234 
but in Spark 2.3 and earlier it is empty in the DROPMALFORMED mode. To restore 
the previous behavior, set `spark.sql.csv.parser.columnPruning.enabled` to 
`false`.
   - Since Spark 2.4, File listing for compute statistics is done in 
parallel by default. This can be disabled by setting 
`spark.sql.parallelFileListingInStatsComputation.enabled` to `False`.
   - Since Spark 2.4, Metadata files (e.g. Parquet summary files) and 
temporary files are not counted as data files when calculating table size 
during Statistics computation.
+  - Since Spark 2.4, empty strings are saved as quoted empty strings `""`. 
In version 2.3 and earlier, empty strings were equal to `null` values and 
didn't reflect to any characters in saved CSV files. For example, the row of 
`"a", null, "", 1` was writted as `a,,,1`. Since Spark 2.4, the same row is 
saved as `a,,"",1`. To restore the previous behavior, set the CSV option 
`emptyValue` to empty (not quoted) string.  
--- End diff --

avoided


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-10 Thread MaxGekk
Github user MaxGekk commented on a diff in the pull request:

https://github.com/apache/spark/pull/22367#discussion_r216324646
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVInferSchema.scala
 ---
@@ -79,7 +79,8 @@ private[csv] object CSVInferSchema {
* point checking if it is an Int, as the final type must be Double or 
higher.
*/
   def inferField(typeSoFar: DataType, field: String, options: CSVOptions): 
DataType = {
-if (field == null || field.isEmpty || field == options.nullValue) {
+if (field == null || field.isEmpty || field == options.nullValue ||
+  field == options.emptyValueInRead) {
--- End diff --

I kept the changes because the test 
https://github.com/apache/spark/pull/22367/files#diff-0433a2d4247fdef1fc57ab95d99218cfR108
 seems reasonable for me. Without the changes it fails since inferred type 
becomes StringType. In any case I will revert this if you insist.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-09 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/22367#discussion_r216196589
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVInferSchema.scala
 ---
@@ -79,7 +79,8 @@ private[csv] object CSVInferSchema {
* point checking if it is an Int, as the final type must be Double or 
higher.
*/
   def inferField(typeSoFar: DataType, field: String, options: CSVOptions): 
DataType = {
-if (field == null || field.isEmpty || field == options.nullValue) {
+if (field == null || field.isEmpty || field == options.nullValue ||
+  field == options.emptyValueInRead) {
--- End diff --

Also, this will exclude empty strings from type inference. Wonder if that 
makes sense. Let's target this change for 3.0.0 if you feel in the similar way.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-09 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/22367#discussion_r216196505
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala
 ---
@@ -91,9 +91,10 @@ abstract class CSVDataSource extends Serializable {
   }
 
   row.zipWithIndex.map { case (value, index) =>
-if (value == null || value.isEmpty || value == options.nullValue) {
-  // When there are empty strings or the values set in 
`nullValue`, put the
-  // index as the suffix.
+if (value == null || value.isEmpty || value == options.nullValue ||
+  value == options.emptyValueInRead) {
--- End diff --

@MaxGekk, can we get rid of this one (and the one in 
`CSVInferSchema.scala`) for now since we target 2.4? IIRC (need to double 
check), this behaviour by `makeSafeHeader` is from R's `read.csv`. We should 
check if it is coherent or not.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-09 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/22367#discussion_r216186604
  
--- Diff: docs/sql-programming-guide.md ---
@@ -1897,6 +1897,7 @@ working with timestamps in `pandas_udf`s to get the 
best performance, see
   - In version 2.3 and earlier, CSV rows are considered as malformed if at 
least one column value in the row is malformed. CSV parser dropped such rows in 
the DROPMALFORMED mode or outputs an error in the FAILFAST mode. Since Spark 
2.4, CSV row is considered as malformed only when it contains malformed column 
values requested from CSV datasource, other values can be ignored. As an 
example, CSV file contains the "id,name" header and one row "1234". In Spark 
2.4, selection of the id column consists of a row with one column value 1234 
but in Spark 2.3 and earlier it is empty in the DROPMALFORMED mode. To restore 
the previous behavior, set `spark.sql.csv.parser.columnPruning.enabled` to 
`false`.
   - Since Spark 2.4, File listing for compute statistics is done in 
parallel by default. This can be disabled by setting 
`spark.sql.parallelFileListingInStatsComputation.enabled` to `False`.
   - Since Spark 2.4, Metadata files (e.g. Parquet summary files) and 
temporary files are not counted as data files when calculating table size 
during Statistics computation.
+  - Since Spark 2.4, empty strings are saved as quoted empty strings `""`. 
In version 2.3 and earlier, empty strings were equal to `null` values and 
didn't reflect to any characters in saved CSV files. For example, the row of 
`"a", null, "", 1` was writted as `a,,,1`. Since Spark 2.4, the same row is 
saved as `a,,"",1`. To restore the previous behavior, set the CSV option 
`emptyValue` to empty (not quoted) string.  
--- End diff --

This fix sounds rather a bug fix tho.. I don't feel strongly documenting 
this ..  but I am okay with it.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-09 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/22367#discussion_r216185993
  
--- Diff: docs/sql-programming-guide.md ---
@@ -1897,6 +1897,7 @@ working with timestamps in `pandas_udf`s to get the 
best performance, see
   - In version 2.3 and earlier, CSV rows are considered as malformed if at 
least one column value in the row is malformed. CSV parser dropped such rows in 
the DROPMALFORMED mode or outputs an error in the FAILFAST mode. Since Spark 
2.4, CSV row is considered as malformed only when it contains malformed column 
values requested from CSV datasource, other values can be ignored. As an 
example, CSV file contains the "id,name" header and one row "1234". In Spark 
2.4, selection of the id column consists of a row with one column value 1234 
but in Spark 2.3 and earlier it is empty in the DROPMALFORMED mode. To restore 
the previous behavior, set `spark.sql.csv.parser.columnPruning.enabled` to 
`false`.
   - Since Spark 2.4, File listing for compute statistics is done in 
parallel by default. This can be disabled by setting 
`spark.sql.parallelFileListingInStatsComputation.enabled` to `False`.
   - Since Spark 2.4, Metadata files (e.g. Parquet summary files) and 
temporary files are not counted as data files when calculating table size 
during Statistics computation.
+  - Since Spark 2.4, empty strings are saved as quoted empty strings `""`. 
In version 2.3 and earlier, empty strings were equal to `null` values and 
didn't reflect to any characters in saved CSV files. For example, the row of 
`"a", null, "", 1` was writted as `a,,,1`. Since Spark 2.4, the same row is 
saved as `a,,"",1`. To restore the previous behavior, set the CSV option 
`emptyValue` to empty (not quoted) string.  
--- End diff --

Not a big deal at all but i would avoid abbreviation (`didn't`) in the 
documentation personally. 


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...

2018-09-08 Thread MaxGekk
GitHub user MaxGekk opened a pull request:

https://github.com/apache/spark/pull/22367

[SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix empty string being parsed as 
null when nullValue is set.

## What changes were proposed in this pull request?

In the PR, I propose new CSV option `emptyValue` and an update in the SQL 
Migration Guide which describes how to revert previous behavior when empty 
strings were not written at all. Since Spark 2.4, empty strings are saved as 
`""` to distinguish them from saved `null`s.

## How was this patch tested?

It was tested by `CSVSuite` and new tests added in the PR #22234


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/MaxGekk/spark-1 csv-empty-value-2.4

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/22367.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 #22367


commit 465ed7a6011bd0437c7f88cb4c18ecea68cb60ac
Author: Mario Molina 
Date:   2018-08-25T17:42:03Z

Configurable empty values when reading/writing CSV files

commit 48e143d43a876afc4f0099bf7079130d74ebe855
Author: Mario Molina 
Date:   2018-08-26T23:29:32Z

Adding tests

commit 70e217146962186a391227f1417cf79c5e81c380
Author: Mario Molina 
Date:   2018-08-26T23:33:55Z

Changing emptyValue order arg in streaming.py

commit 8665f93c442915dc23a40ffb3c958a097dec34c5
Author: Mario Molina 
Date:   2018-08-27T02:03:41Z

Changing emptyValue order arg in set_opts

commit 867c6de34673bbc877e0e26e8c0d662e038e2946
Author: Maxim Gekk 
Date:   2018-09-08T20:40:41Z

Added comments for parameters

commit e0cb879f3bc28f66e19d049ed0ee6dc33fc5922c
Author: Maxim Gekk 
Date:   2018-09-08T21:02:21Z

Updating the migration guide

commit e23098c5a6322ab3cff851b37889163c9bd09491
Author: Mario Molina 
Date:   2018-08-26T23:28:34Z

Changing order in args for emptyValue

commit 732ec78c8d376bad0cc8897b1da48a56448590fb
Author: Maxim Gekk 
Date:   2018-09-08T21:11:56Z

Revert some checking

commit 7eac385568c78735bb7743cfcfa234c4bea97fb0
Author: Maxim Gekk 
Date:   2018-09-08T21:14:13Z

Revert unneeded changes




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org