Skip to content

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Nov 25, 2019

What changes were proposed in this pull request?

Running SQLQueryTestSuite for testing, i.e.

 SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/test-only *SQLQueryTestSuite -- -z cast.sql"

will fail with hive2.3 dependecies. Because the hadoop version used here is 2.7.4 by default.

....
....
....

[error] /Users/kentyao/spark/sql/core/v2.3/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcShimUtils.scala:64: not found: type HiveDecimalWritable
[error]       new HiveDecimalWritable(HiveDecimal.create(d.toJavaBigDecimal))
[error]           ^
[error] /Users/kentyao/spark/sql/core/v2.3/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcShimUtils.scala:64: not found: value HiveDecimal
[error]       new HiveDecimalWritable(HiveDecimal.create(d.toJavaBigDecimal))
[error]                               ^
[warn] two warnings found
[error] 39 errors found
[error] (sql/compile:compileIncremental) Compilation failed
[error] Total time: 64 s, completed 2019-11-25 13:22:12

This pr is a followup, only update the doc to run SQLQueryTestSuite correctly.

SPARK_GENERATE_GOLDEN_FILES=1 build/sbt -Phive-1.2 "sql/test-only *SQLQueryTestSuite -- -z cast.sql"
[info] ScalaTest
[info] Run completed in 10 seconds, 191 milliseconds.
[info] Total number of tests run: 1
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[info] Passed: Total 1, Failed 0, Errors 0, Passed 1
[success] Total time: 85 s, completed 2019-11-25 13:29:34

Why are the changes needed?

the default scripts fail to run tests because the underlying profile changed.

Does this PR introduce any user-facing change?

no. developer side change.

How was this patch tested?

existing ut

@yaooqinn
Copy link
Member Author

cc @dongjoon-hyun, thanks for reviewing

@maropu
Copy link
Member

maropu commented Nov 25, 2019

What's the error happend w/o that profile? Can you decribe more in the PR description?

* To run the entire test suite:
* {{{
* build/sbt "sql/test-only *SQLQueryTestSuite"
* build/sbt -Phive-1.2 "sql/test-only *SQLQueryTestSuite"
Copy link
Member

Choose a reason for hiding this comment

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

First of all, we had better use -Phive-2.3.
Second, please wait for a while. This PR will become obsolete soon.
As you see, it's still under slow changing to avoid any breakage.
Yesterday, we added hive-1.2/2.3. Today, we regenerates the manifest via test-dependencies.sh.
And, I'm working on the dependency changes. The following is the history of the migration.

a1706e2 [SPARK-30005][INFRA] Update test-dependencies.sh to check hive-1.2/2.3 profile
a60da23 [SPARK-30007][INFRA] Publish snapshot/release artifacts with -Phive-2.3 only
13338ea [SPARK-29554][SQL][FOLLOWUP] Rename Version to SparkVersion
6625b69 [SPARK-29981][BUILD][FOLLOWUP] Change hive.version.short
c98e5eb [SPARK-29981][BUILD] Add hive-1.2/2.3 profiles

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the instructions

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for understanding and sorry for the inconvenience. It's difficult to change the tires of the running cars.

Copy link
Member Author

Choose a reason for hiding this comment

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

Never mind. I will leave this pull request open for a while in case someone encounters the same issue until it is completely fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Although I'm still working on, I create a WIP PR to share with you.

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.

To sum up, let's hold on this for a few days.

@dongjoon-hyun
Copy link
Member

BTW, it's described on the main PR.
sql/core module requires -Phive-1.2 or -Phive-2.3 for now in order to bring the extra dependency correctly which was under -Phadoop-3.2 profile before.

@SparkQA
Copy link

SparkQA commented Nov 25, 2019

Test build #114372 has finished for PR 26657 at commit 2e7183f.

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

* -z describe.sql"
* }}}
*
* Change profiles:
Copy link
Member

Choose a reason for hiding this comment

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

Plus about the default profile (and it will become obsolete), such documentation should better go to somewhere related to Spark build or testing.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

Will we even change this? We will have a default profile anyway, and make it working.
Even if we need this, I think it has to be a part of a PR that targets a bigger scope.

Shall we close this and cherry-pick in a bigger PR if needed in any event? credits will be co-authored to everybody involved in commits in a PR anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants