Skip to content

Conversation

@HyukjinKwon
Copy link
Member

What changes were proposed in this pull request?

This PR adds prettyNames for from_json, to_json, from_csv, and schema_of_json so that appropriate names are used.

How was this patch tested?

Unit tests

@SparkQA
Copy link

SparkQA commented Oct 19, 2018

Test build #97591 has finished for PR 22773 at commit 3447e73.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

retest this please

@SparkQA
Copy link

SparkQA commented Oct 19, 2018

Test build #97599 has finished for PR 22773 at commit 3447e73.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya
Copy link
Member

viirya commented Oct 19, 2018

retest this please.

@viirya
Copy link
Member

viirya commented Oct 19, 2018

LGTM

@SparkQA
Copy link

SparkQA commented Oct 19, 2018

Test build #97604 has finished for PR 22773 at commit 3447e73.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

LG, but could you make this as a follow-up for the existing issues or create a new one?

@HyukjinKwon
Copy link
Member Author

Other JIRAs have different fixed versions. Let me create a new JIRA then.

@HyukjinKwon HyukjinKwon changed the title [MINOR][SQL] Add prettyNames for from_json, to_json, from_csv, and schema_of_json [SPARK-25785][SQL] Add prettyNames for from_json, to_json, from_csv, and schema_of_json Oct 20, 2018
@HyukjinKwon
Copy link
Member Author

Merged to master.

@HyukjinKwon
Copy link
Member Author

Thank you @viirya and @dongjoon-hyun.

@asfgit asfgit closed this in 3370865 Oct 20, 2018
@gatorsmile
Copy link
Member

gatorsmile commented Oct 25, 2018

This is an external change. This needs a migration guide update. Column names in schema are changed by this PR

@HyukjinKwon
Copy link
Member Author

That's the exact issue I raised before and we ended up with not keeping the compatibility in column names. @cloud-fan and @hvanhovell.

@HyukjinKwon
Copy link
Member Author

BTW, it's closer to bug rather then improvement tho. from_json should have default name from_json rather then jsontostructs - end users would have no idea why it's called jsontostructs.

@cloud-fan
Copy link
Contributor

I think the new names are better and expected, though it's safer to mention it in the migration guide in case some users care about it.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Oct 25, 2018

My impression so far was that we note things at migration notes when they are improvements (not bugs), and non-trivial and related to backward compatibility.

Shall we clarify what to document at migration guide? Otherwise we should document everything related with, let's say, all external changes, deprecation removal, trivial changes, and many other changes related with column names.

@gatorsmile
Copy link
Member

All the things that could break the existing user applications should be documented in the migration guide. This will simplify the system upgrade of our end users.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Oct 25, 2018

Sure, so for clarification, we will document everything that could break external users applications in any way, right?

@gatorsmile
Copy link
Member

Yes. The goal of migration guide is for helping end users upgrade their Spark.

@HyukjinKwon
Copy link
Member Author

Yup, will encourage to update the migration guide in that way.

jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
…and schema_of_json

## What changes were proposed in this pull request?

This PR adds `prettyNames` for `from_json`, `to_json`, `from_csv`, and `schema_of_json` so that appropriate names are used.

## How was this patch tested?

Unit tests

Closes apache#22773 from HyukjinKwon/minor-prettyNames.

Authored-by: hyukjinkwon <[email protected]>
Signed-off-by: hyukjinkwon <[email protected]>
@HyukjinKwon HyukjinKwon deleted the minor-prettyNames branch March 3, 2020 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants