Skip to content

Conversation

@jinxing64
Copy link

What changes were proposed in this pull request?

This pr address comments in #19868 ;
Fix the code style for org.apache.spark.sql.hive.QueryPartitionSuite by using:
withTempView, withTempDir, withTable...

@jinxing64
Copy link
Author

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Apr 18, 2018

Test build #89481 has finished for PR 21091 at commit fd099bf.

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

@jinxing64
Copy link
Author

cc @cloud-fan


sql("DROP TABLE IF EXISTS table_with_partition")
sql("DROP TABLE IF EXISTS createAndInsertTest")
withSQLConf((SQLConf.HIVE_VERIFY_PARTITION_PATH.key -> "true")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we only need one pair of ()

withTempView("testData") {
withTable("table_with_partition", "createAndInsertTest") {
withTempPath { tmpDir =>
tmpDir.mkdir()
Copy link
Contributor

Choose a reason for hiding this comment

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

withTempDir, then you don't need to call mkdir here


// test for the exist path
checkAnswer(sql("select key,value from table_with_partition"),
testData.union(testData).union(testData).union(testData).collect)
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to call collect, checkAnswer can compare 2 dataframes

@jinxing64 jinxing64 force-pushed the SPARK-22676-FOLLOW-UP branch from 59edf82 to e9ff4d0 Compare April 19, 2018 02:56
@SparkQA
Copy link

SparkQA commented Apr 19, 2018

Test build #89543 has finished for PR 21091 at commit e9ff4d0.

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

@SparkQA
Copy link

SparkQA commented Apr 19, 2018

Test build #89541 has finished for PR 21091 at commit 59edf82.

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

}
}

test("Replace spark.sql.hive.verifyPartitionPath by spark.files.ignoreMissingFiles") {
Copy link
Contributor

Choose a reason for hiding this comment

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

These 2 tests are exactly same except the config, can we create a common method for it?

Copy link
Author

Choose a reason for hiding this comment

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

I extracted a common method, check again.

@jinxing64 jinxing64 force-pushed the SPARK-22676-FOLLOW-UP branch from 4838b79 to e9ff4d0 Compare April 19, 2018 08:50
@SparkQA
Copy link

SparkQA commented Apr 19, 2018

Test build #89562 has finished for PR 21091 at commit 3bf4536.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@asfgit asfgit closed this in 9e10f69 Apr 19, 2018
@jinxing64
Copy link
Author

Thanks for merging !

@jinxing64 jinxing64 deleted the SPARK-22676-FOLLOW-UP branch April 20, 2018 02:29
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