-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-22416][SQL] Move OrcOptions from sql/hive to sql/core
#19636
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
Conversation
|
Test build #83306 has started for PR 19636 at commit |
|
Retest this please. |
|
Test build #83308 has started for PR 19636 at commit |
| * Options for the ORC data source. | ||
| */ | ||
| private[orc] class OrcOptions( | ||
| private[sql] class OrcOptions( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe private[sql] can be removed per SPARK-16964.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @HyukjinKwon !
|
Retest this please. |
1 similar comment
|
Retest this please. |
jiangxb1987
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Thank you for review, @jiangxb1987 . |
|
Test build #83317 has finished for PR 19636 at commit
|
|
Hi, @cloud-fan and @gatorsmile . |
|
LGTM, merging to master! |
|
Thank you so much, @cloud-fan , @HyukjinKwon , and @jiangxb1987 ! |
What changes were proposed in this pull request?
According to the discussion on SPARK-15474, we will add new OrcFileFormat in
sql/coremodule and allow users to use both old and new OrcFileFormat.To do that,
OrcOptionsshould be visible insql/coremodule, too. Previously, it wasprivate[orc]insql/hive. This PR removesprivate[orc]because we don't useprivate[sql]insql/executionpackage after SPARK-16964.How was this patch tested?
Pass the Jenkins with the existing tests.