Skip to content

[SPARK-33082][SQL] Remove hive-1.2 workaround code #29961

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
wants to merge 2 commits into from
Closed

[SPARK-33082][SQL] Remove hive-1.2 workaround code #29961

wants to merge 2 commits into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Oct 7, 2020 •
edited
Loading

What changes were proposed in this pull request?

This PR removes old Hive-1.2 profile related workaround code.

Why are the changes needed?

To simply the code.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the CI.

@dongjoon-hyun dongjoon-hyun marked this pull request as draft October 7, 2020 05:37
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-XXX][SQL] Remove hive-1.2 workaround code [SPARK-33082][SQL] Remove hive-1.2 workaround code Oct 7, 2020
@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34097/

@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34097/

@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Test build #129491 has finished for PR 29961 at commit 08edebc.

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

@HeartSaVioR
Copy link
Contributor

retest this, please

@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34100/

@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34100/

@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Test build #129494 has finished for PR 29961 at commit 08edebc.

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

@dongjoon-hyun dongjoon-hyun marked this pull request as ready for review October 7, 2020 16:09
@dongjoon-hyun
Copy link
Member Author

Thank you for re-trigger, @HeartSaVioR !

@dongjoon-hyun
Copy link
Member Author 

KubernetesSuite:
- Run SparkPi with no resources *** FAILED ***
...
Tests: succeeded 17, failed 1, canceled 0, ignored 0, pending 0

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Oct 7, 2020 •
edited
Loading

This PR is ready. Could you review this PR?
@wangyum , @HyukjinKwon , @maropu , @viirya , @dbtsai , @cloud-fan , @HeartSaVioR , @gengliangwang .

cloud-fan
cloud-fan reviewed Oct 7, 2020
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala Outdated
@@ -581,7 +580,7 @@ class OrcSourceSuite extends OrcSuite with SharedSparkSession {
}

test("Enforce direct encoding column-wise selectively") {
testSelectiveDictionaryEncoding(isSelective = true)
testSelectiveDictionaryEncoding(isSelective = true, isHive23 = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

why isHive23 is false here?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Oct 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is sql/core module. This test case is using Apache ORC directly.

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Oct 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This test case was added originally via [SPARK-25635][SQL][BUILD] Support selective direct encoding in native ORC write .

Copy link
Member Author

Choose a reason for hiding this comment

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

After that, [SPARK-27402][SQL][TEST-HADOOP3.2][TEST-MAVEN] Fix hadoop-3.2 test issue(except the hive-thriftserver module changed like the following.

- protected def testSelectiveDictionaryEncoding(isSelective: Boolean) {
+ protected def testSelectiveDictionaryEncoding(isSelective: Boolean, isHive23: Boolean = false) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially, SPARK-27402 seems to use isHive23 misleadingly because isHive23=false means Apache ORC in sql/core module and Hive 1.2.1 fork in sql/hive module.

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @wangyum and @gatorsmile

Copy link
Contributor

Choose a reason for hiding this comment

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

shall we rename to isHiveOrc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure!

@dongjoon-hyun
Copy link
Member Author

Thank you, @cloud-fan ! The last commit is renaming only and the compilation passed already in Jenkins.
Merged to master for Apache Spark 3.1.0.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-HIVE12 branch October 7, 2020 19:28
@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34132/

@SparkQA
Copy link

SparkQA commented Oct 7, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34132/

@SparkQA
Copy link

SparkQA commented Oct 8, 2020

Test build #129527 has finished for PR 29961 at commit 071db3b.

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

@c21 c21 mentioned this pull request Oct 11, 2020
Closed
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 approved these changes

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
@dongjoon-hyun @SparkQA @HeartSaVioR @cloud-fan