Skip to content
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.

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

[SPARK-32234][FOLLOWUP][SQL]Update the description of utility method #29232

Closed

Conversation

SaurabhChawla100
Copy link
Contributor

What changes were proposed in this pull request?

As the part of this PR #29045 added the helper method. This PR is the FOLLOWUP PR to update the description of helper method.

Why are the changes needed?

For better readability and understanding of the code

Does this PR introduce any user-facing change?

No

How was this patch tested?

Since its only change of updating the description , So ran the Spark shell

@SaurabhChawla100
Copy link
Contributor Author

@SaurabhChawla100 SaurabhChawla100 changed the title [SPARK-32234][FOLLOWUP][SQL]Update the description utility method [SPARK-32234][FOLLOWUP][SQL]Update the description of utility method Jul 25, 2020
@dongjoon-hyun
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Jul 25, 2020

Test build #126541 has finished for PR 29232 at commit 80ccedd.

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

* resultSchemaString will be created using pruned col in case of
* canPruneCols is true and for canPruneCols as false value
* resultSchemaString will be created using the actual dataSchema.
* Also as the part of this method update the MAPRED_INPUT_SCHEMA
Copy link
Contributor

Choose a reason for hiding this comment

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

update the MAPRED_INPUT_SCHEMA -> updates the MAPRED_INPUT_SCHEMA in the given 'conf'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the comment as suggested by @HyukjinKwon

* resultSchemaString will be created using resultsSchema in case of
* canPruneCols is true and for canPruneCols as false value
* resultSchemaString will be created using the actual dataSchema.
* This method returns the result schema as string based on the canPruneCols flag.
Copy link
Member

Choose a reason for hiding this comment

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

I would just write as:

Returns the result schema to read from ORC file. In addition, It sets the schema string to 'orc.mapred.input.schema' so ORC readers can use later.

and inline the details about parameters into the parameter descriptions below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that looks fine. Will update as suggested

@SparkQA
Copy link

SparkQA commented Jul 27, 2020

Test build #126609 has finished for PR 29232 at commit 54bba19.

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

@HyukjinKwon
Copy link
Member

retest this please

@cloud-fan
Copy link
Contributor

github action passes, merging to master, thanks!

@cloud-fan cloud-fan closed this in 99f33ec Jul 27, 2020
@SparkQA
Copy link

SparkQA commented Jul 27, 2020

Test build #126628 has finished for PR 29232 at commit 54bba19.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants