Skip to content

Conversation

@yjshen
Copy link
Member

@yjshen yjshen commented Jul 10, 2015

@yjshen
Copy link
Member Author

yjshen commented Jul 10, 2015

@yhuai it seems not possible to do checking in CheckAnalysis because LogicalRelation or LogicalRDD is not accessible there?

@yhuai
Copy link
Contributor

yhuai commented Jul 10, 2015

You can put it in org.apache.spark.sql.sources.PreWriteCheck (it is a extendedCheckRules configured in SQLContext and HiveContext), which is located in sql/core. Eventually, we will move it out from the package of sources since this is another round of write related checks.

@yjshen
Copy link
Member Author

yjshen commented Jul 12, 2015

thanks, I would update my implementation :)

Copy link
Contributor

Choose a reason for hiding this comment

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

"Inserting into an RDD-based table is not allowed."

@yhuai
Copy link
Contributor

yhuai commented Jul 13, 2015

ok to test

@SparkQA
Copy link

SparkQA commented Jul 13, 2015

Test build #37119 has finished for PR 7342 at commit 68324a1.

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

@SparkQA
Copy link

SparkQA commented Jul 13, 2015

Test build #1052 has finished for PR 7342 at commit 68324a1.

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

@SparkQA
Copy link

SparkQA commented Jul 14, 2015

Test build #37225 has finished for PR 7342 at commit 0c635d4.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

fix import order here

@SparkQA
Copy link

SparkQA commented Jul 15, 2015

Test build #37303 has finished for PR 7342 at commit 09518af.

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

@rxin
Copy link
Contributor

rxin commented Jul 15, 2015

Jenkins, retest this please.

Copy link
Contributor

Choose a reason for hiding this comment

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

For this test suite, can you change class DataFrameSuite extends QueryTest { to class DataFrameSuite extends QueryTest with SQLTestUtils {? Then, you can use withTempPath to create a temp dir and save data in this temp dir. withTempPath will automatically clean up all of your temp data/dirs. Basically, you can do

test("SPARK-6941: Better error message for inserting into RDD-based Table") {
  withTempPath { dir =>
    // You can create some dirs inside the given temp dir.
    val tempParquet = new File(dir, "tmp_parquet")
    val tempJson = new File(dir, "tmp_json")
    ...
    // Your test code, which uses created temp dirs.
    ...
  }
}

@yhuai
Copy link
Contributor

yhuai commented Jul 15, 2015

Looks good. Left a few comments about the test.

@SparkQA
Copy link

SparkQA commented Jul 15, 2015

Test build #37394 has finished for PR 7342 at commit 09518af.

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

@yhuai
Copy link
Contributor

yhuai commented Jul 16, 2015

Thanks for the update! LGTM. I will merge it to master once jenkins passes.

@SparkQA
Copy link

SparkQA commented Jul 16, 2015

Test build #37513 has finished for PR 7342 at commit 3d3f5dc.

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

@asfgit asfgit closed this in 43dac2c Jul 16, 2015
@SparkQA
Copy link

SparkQA commented Jul 16, 2015

Test build #37516 has finished for PR 7342 at commit f82cbe7.

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

@yjshen
Copy link
Member Author

yjshen commented Jul 16, 2015

@yhuai, Thanks for the step by step guide, really appreciate it.

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