Skip to content

[SPARK-19695][SQL] Throw an exception if a columnNameOfCorruptRecord field violates requirements in json formats #17023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
maropu wants to merge 2 commits into from
Closed

[SPARK-19695][SQL] Throw an exception if a columnNameOfCorruptRecord field violates requirements in json formats #17023

maropu wants to merge 2 commits into from

Conversation

@maropu
Copy link
Member

@maropu maropu commented Feb 22, 2017

What changes were proposed in this pull request?

This pr comes from #16928 and fixed a json behaviour along with the CSV one.

How was this patch tested?

Added tests in JsonSuite.

@maropu maropu changed the title [SPARK-19695][SQL] Throw an exception if a columnNameOfCorruptRecord field violates requirements [SPARK-19695][SQL] Throw an exception if a columnNameOfCorruptRecord field violates requirements for json Feb 22, 2017
@maropu maropu changed the title [SPARK-19695][SQL] Throw an exception if a columnNameOfCorruptRecord field violates requirements for json [SPARK-19695][SQL] Throw an exception if a columnNameOfCorruptRecord field violates requirements in json formats Feb 22, 2017
@maropu maropu mentioned this pull request Feb 22, 2017
Closed
@SparkQA
Copy link

SparkQA commented Feb 22, 2017

Test build #73259 has finished for PR 17023 at commit 11c2850.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member Author

maropu commented Feb 22, 2017

@HyukjinKwon @cloud-fan cloud you check this? thanks.

cloud-fan
cloud-fan reviewed Feb 22, 2017
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/json/JsonSuite.scala Outdated
Copy link
Contributor

@cloud-fan cloud-fan Feb 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corruptRecords.toDF("value")?

HyukjinKwon
HyukjinKwon reviewed Feb 22, 2017
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/json/JsonFileFormat.scala Outdated
Copy link
Member

@HyukjinKwon HyukjinKwon Feb 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map to foreach?

Copy link
Member

@HyukjinKwon HyukjinKwon Feb 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And.. not a strong preference but maybe put this in JacksonUtils and rename it JsonUtils if @cloud-fan is okay?

Maybe we could throws an exception as IlligalArgumentException in the first place and then capture the message with AnalysisException (as JacksonUtils.verifySchema is doing in StructToJson). This is not a strong opinion too.

Copy link
Contributor

@cloud-fan cloud-fan Feb 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally the columnNameOfCorruptRecord stuff has nothing to do with parser. Parser just parses the record and report error if some records are bad, and the upper-level will handle the bad records and may put the bad record in a special string column.

I'm ok to keep this code snippet duplicated in 2 places.

Copy link
Member Author

@maropu maropu Feb 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay!

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Feb 22, 2017

Thanks for cc'ing me. I am okay if @cloud-fan is okay.

@cloud-fan
Copy link
Contributor

cloud-fan commented Feb 23, 2017

LGTM

@SparkQA
Copy link

SparkQA commented Feb 23, 2017

Test build #73312 has finished for PR 17023 at commit 00e0b7a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

cloud-fan commented Feb 23, 2017

thanks, merging to master!

@asfgit asfgit closed this in 769aa0f Feb 23, 2017
Yunni pushed a commit to Yunni/spark that referenced this pull request Feb 27, 2017
@maropu
[SPARK-19695][SQL] Throw an exception if a `columnNameOfCorruptRecord…
46f4a19 
…` field violates requirements in json formats

## What changes were proposed in this pull request?
This pr comes from apache#16928 and fixed a json behaviour along with the CSV one.

## How was this patch tested?
Added tests in `JsonSuite`.

Author: Takeshi Yamamuro <[email protected]>

Closes apache#17023 from maropu/SPARK-19695.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cloud-fan cloud-fan cloud-fan left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maropu @SparkQA @HyukjinKwon @cloud-fan