[SPARK-23173][SQL] Avoid creating corrupt parquet files when loading data from JSON #20694
Conversation
This file contains hidden or 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
Member
|
ok to test |
|
Test build #87776 has finished for PR 20694 at commit
|
gatorsmile
reviewed
Feb 28, 2018
| test("from_json missing fields") { | ||
| val conf = SQLConf.get | ||
| for (forceJsonNullableSchema <- Seq(false, true)) { | ||
| conf.setConf(SQLConf.FROM_JSON_FORCE_NULLABLE_SCHEMA, forceJsonNullableSchema) |
Member
There was a problem hiding this comment.
We have to revert the conflicts to the original value.
Thus, move this test to org.apache.spark.sql.execution.datasources.json.JsonSuite. Then, we can use SQLConf
Contributor
Author
There was a problem hiding this comment.
I tried moving the test, but it is not as easy as it may seem. org.apache.spark.sql.execution.datasources.json.JsonSuite lacks checkEvaluation that I'm using here. In general, org.apache.spark.sql.execution.datasources.json.JsonSuite seems to be meant for things such as spark.read.json() and not the from_json() function, so the test would be misplaced there. I think that it's better to keep the test in JsonExpressionsSuite and revert the config.
Member
|
LGTM except one minor comment in the test case. |
|
Test build #88121 has finished for PR 20694 at commit
|
Member
|
retest this please |
|
Test build #88126 has finished for PR 20694 at commit
|
gatorsmile
reviewed
Mar 9, 2018
| val schema = JsonToStructs(jsonSchema, Map.empty, Literal.create(input, StringType), gmtId) | ||
| .dataType | ||
| val schemaToCompare = if (forceJsonNullableSchema) jsonSchema.asNullable else jsonSchema | ||
| assert(schemaToCompare == schema); |
gatorsmile
reviewed
Mar 9, 2018
| | "c": "foo" | ||
| |} | ||
| |""" | ||
| .stripMargin |
gatorsmile
reviewed
Mar 9, 2018
| | "a": 1, | ||
| | "c": "foo" | ||
| |} | ||
| |""" |
Member
|
LGTM Thanks! Merged to master/2.3 I resolved the style issues when I merged the code |
asfgit
pushed a commit
that referenced
this pull request
Mar 9, 2018
…data from JSON ## What changes were proposed in this pull request? The from_json() function accepts an additional parameter, where the user might specify the schema. The issue is that the specified schema might not be compatible with data. In particular, the JSON data might be missing data for fields declared as non-nullable in the schema. The from_json() function does not verify the data against such errors. When data with missing fields is sent to the parquet encoder, there is no verification either. The end results is a corrupt parquet file. To avoid corruptions, make sure that all fields in the user-specified schema are set to be nullable. Since this changes the behavior of a public function, we need to include it in release notes. The behavior can be reverted by setting `spark.sql.fromJsonForceNullableSchema=false` ## How was this patch tested? Added two new tests. Author: Michał Świtakowski <[email protected]> Closes #20694 from mswit-databricks/SPARK-23173. (cherry picked from commit 2ca9bb0) Signed-off-by: gatorsmile <[email protected]>
peter-toth
pushed a commit
to peter-toth/spark
that referenced
this pull request
Oct 6, 2018
…data from JSON ## What changes were proposed in this pull request? The from_json() function accepts an additional parameter, where the user might specify the schema. The issue is that the specified schema might not be compatible with data. In particular, the JSON data might be missing data for fields declared as non-nullable in the schema. The from_json() function does not verify the data against such errors. When data with missing fields is sent to the parquet encoder, there is no verification either. The end results is a corrupt parquet file. To avoid corruptions, make sure that all fields in the user-specified schema are set to be nullable. Since this changes the behavior of a public function, we need to include it in release notes. The behavior can be reverted by setting `spark.sql.fromJsonForceNullableSchema=false` ## How was this patch tested? Added two new tests. Author: Michał Świtakowski <[email protected]> Closes apache#20694 from mswit-databricks/SPARK-23173. (cherry picked from commit 2ca9bb0) Signed-off-by: gatorsmile <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
What changes were proposed in this pull request?
The from_json() function accepts an additional parameter, where the user might specify the schema. The issue is that the specified schema might not be compatible with data. In particular, the JSON data might be missing data for fields declared as non-nullable in the schema. The from_json() function does not verify the data against such errors. When data with missing fields is sent to the parquet encoder, there is no verification either. The end results is a corrupt parquet file.
To avoid corruptions, make sure that all fields in the user-specified schema are set to be nullable.
Since this changes the behavior of a public function, we need to include it in release notes.
The behavior can be reverted by setting
spark.sql.fromJsonForceNullableSchema=falseHow was this patch tested?
Added two new tests.