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-22672][TEST][SQL] Move OrcTest to sql/core #19863

Closed
wants to merge 1 commit into from
Closed

[SPARK-22672][TEST][SQL] Move OrcTest to sql/core #19863

wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

What changes were proposed in this pull request?

To support ORC tests without Hive, we had better have OrcTest in sql/core instead of sql/hive.

How was this patch tested?

This is a test suite only change. It should pass all existing tests.

@SparkQA
Copy link

SparkQA commented Dec 2, 2017

Test build #84381 has finished for PR 19863 at commit a58ceba.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • abstract class OrcTest extends QueryTest with SQLTestUtils
  • class OrcFilterSuite extends OrcTest with TestHiveSingleton
  • class OrcQuerySuite extends OrcTest with TestHiveSingleton with BeforeAndAfterAll

import org.apache.spark.sql.test.SQLTestUtils

private[sql] trait OrcTest extends SQLTestUtils with TestHiveSingleton {
abstract class OrcTest extends QueryTest with SQLTestUtils {
Copy link
Member

@gatorsmile gatorsmile Dec 2, 2017

Choose a reason for hiding this comment

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

When we have not moved the test suites of ORC, why we need to move this out of Hive now?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Dec 2, 2017

Choose a reason for hiding this comment

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

Unlike the previous ORC PRs, #19651 intentionally uses the existing old Hive tests suites. This PR is a preparation for moving test cases.

Copy link
Member

Choose a reason for hiding this comment

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

Since this PR is tiny, we can do it when the other PRs are merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

As you see, we can do this without merging other ORC PRs, and this will reduce the scope of review for the other PRs.

@dongjoon-hyun
Copy link
Member Author

@gatorsmile . Since the main PR is merged, I'll include this into the others. Thanks!

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.

3 participants