Skip to content

Conversation

@windpiger
Copy link
Contributor

@windpiger windpiger commented Feb 24, 2017

What changes were proposed in this pull request?

This JIRA is a follow up work after SPARK-19583

As we discussed in that PR

The following DDL for datasource table with an non-existent location should work:

CREATE TABLE ... (PARTITIONED BY ...) LOCATION path

Currently it will throw exception that path not exists for datasource table for datasource table

How was this patch tested?

unit test added

@SparkQA
Copy link

SparkQA commented Feb 24, 2017

Test build #73408 has started for PR 17055 at commit 89eb03a.

@windpiger
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Feb 24, 2017

Test build #73415 has finished for PR 17055 at commit 89eb03a.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

withTable("t", "t1") {
withTempDir {
dir =>
dir.delete()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are lots of dir existed test case in DDLSuit or HiveDDLSuit, so we just add non-existent test cases

@windpiger
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Feb 24, 2017

Test build #73427 has finished for PR 17055 at commit 89eb03a.

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

@windpiger
Copy link
Contributor Author

windpiger commented Feb 26, 2017

cc @gatorsmile @cloud-fan I will appreciate that you could help to review this pr~

@windpiger
Copy link
Contributor Author

ping @gatorsmile

@gatorsmile
Copy link
Member

I will review this PR in the next few days. Thanks!

@windpiger
Copy link
Contributor Author

ok, thanks very much~

@SparkQA
Copy link

SparkQA commented Mar 2, 2017

Test build #73762 has finished for PR 17055 at commit 0e09b92.

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

withTable("t", "t1") {
withTempDir {
dir =>
dir.delete()
Copy link
Member

Choose a reason for hiding this comment

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

withTempPath?

test("create datasource table with a non-existing location") {
withTable("t", "t1") {
withTempDir {
dir =>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: style issue. the above two lines can be combined together.

|CREATE TABLE t(a int, b int)
|USING parquet
|LOCATION '$dir'
""".stripMargin)
Copy link
Member

Choose a reason for hiding this comment

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

The test case is already pretty long. Please reduce the sql statement to a single line.

}
}

test("create datasource table with a non-existing location") {
Copy link
Member

Choose a reason for hiding this comment

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

Let me try to combine the HiveDDLSuite.scala and DDLSuite.scala. Otherwise, the test cases need to be duplicated in every PR.

@SparkQA
Copy link

SparkQA commented Mar 6, 2017

Test build #74003 has finished for PR 17055 at commit d4378dd.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 6, 2017

Test build #74010 has finished for PR 17055 at commit 4c82dfb.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 6, 2017

Test build #74015 has finished for PR 17055 at commit 3ea3e9d.

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

@windpiger
Copy link
Contributor Author

@gatorsmile this pr could be merged?

@SparkQA
Copy link

SparkQA commented Mar 9, 2017

Test build #74261 has finished for PR 17055 at commit bc7f08d.

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

@windpiger
Copy link
Contributor Author

@gatorsmile I have merged with master,if it is ok, could you help to merge it?

@SparkQA
Copy link

SparkQA commented Mar 9, 2017

Test build #74263 has finished for PR 17055 at commit 207448d.

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

@SparkQA
Copy link

SparkQA commented Mar 9, 2017

Test build #74264 has finished for PR 17055 at commit 4024d7c.

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

@gatorsmile
Copy link
Member

I will review it today. Thanks!

@windpiger
Copy link
Contributor Author

OK thanks a lot~

Seq(true, false).foreach { shouldDelete =>
val tcName = if (shouldDelete) "non-existent" else "existed"
val tcName = if (shouldDelete) "non-existing" else "existed"
test(s"CTAS for external data source table with a $tcName location") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@windpiger windpiger Mar 11, 2017

Choose a reason for hiding this comment

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

oh, it is. after we also qualified the location path for datasource table , the code for equal two location are the same with HiveExternalCatalog. thanks~

withTempDir { dir =>
if (shouldDelete) {
dir.delete()
}
Copy link
Member

Choose a reason for hiding this comment

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

if (shouldDelete) dir.delete()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok thanks~~

spark.sql(
s"""
|CREATE TABLE t1(a int, b int) USING parquet PARTITIONED BY(a) LOCATION '$dir'
""".stripMargin)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: one line?

@SparkQA
Copy link

SparkQA commented Mar 11, 2017

Test build #74360 has finished for PR 17055 at commit e1c891e.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 11, 2017

Test build #74361 has finished for PR 17055 at commit ada275c.

  • 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 f6fdf92 Mar 11, 2017
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.

4 participants