[GitHub] spark pull request #22367: [SPARK-17916][SPARK-25241][SQL][FOLLOWUP] Fix emp...
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...
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...
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...
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...
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...
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...
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...
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