[SPARK-16185] [SQL] Better Error Messages When Creating Table As Select Without Enabling Hive Support #13886

gatorsmile · 2016-06-24T07:16:31Z

What changes were proposed in this pull request?

When we do not turn on the Hive Support, the following query generates a confusing error message by Planner:

sql("CREATE TABLE t2 SELECT a, b from t1")

assertion failed: No plan for CreateTable CatalogTable(
    Table: `t2`
    Created: Tue Aug 09 23:45:32 PDT 2016
    Last Access: Wed Dec 31 15:59:59 PST 1969
    Type: MANAGED
    Provider: hive
    Storage(InputFormat: org.apache.hadoop.mapred.TextInputFormat, OutputFormat: org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat)), ErrorIfExists
+- Relation[a#19L,b#20L] parquet

java.lang.AssertionError: assertion failed: No plan for CreateTable CatalogTable(
    Table: `t2`
    Created: Tue Aug 09 23:45:32 PDT 2016
    Last Access: Wed Dec 31 15:59:59 PST 1969
    Type: MANAGED
    Provider: hive
    Storage(InputFormat: org.apache.hadoop.mapred.TextInputFormat, OutputFormat: org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat)), ErrorIfExists
+- Relation[a#19L,b#20L] parquet

This PR is to issue a better error message:

Hive support is required to use CREATE Hive TABLE AS SELECT

How was this patch tested?

Added test cases in DDLSuite.scala

SparkQA · 2016-06-24T08:50:59Z

Test build #61159 has finished for PR 13886 at commit db3d44c.

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

SparkQA · 2016-06-24T09:07:03Z

Test build #61158 has finished for PR 13886 at commit 8947445.

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

SparkQA · 2016-06-24T17:55:01Z

Test build #61180 has finished for PR 13886 at commit e4cc35d.

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

gatorsmile · 2016-06-24T18:13:32Z

cc @cloud-fan @hvanhovell could you please review this simple change? Thanks!

cloud-fan · 2016-06-25T11:17:23Z

hmmm, why can't we support CREATE TABLE AS SELECT without hive support?

gatorsmile · 2016-06-25T16:42:44Z

CREATE TABLE AS SELECT can be converted to Create Data Source Table when the following condition is true: the statement does not have the user-specified file format and row format. (Actually, when the file format is Parquet, we still can convert it. Will try to submit a PR for this case)

However, the default value of internal Conf spark.sql.hive.convertCTAS is false. Thus, we do not convert them even if it is possible. Maybe we can add a rule to do it when users do not enable Hive support?

gatorsmile · 2016-06-26T05:01:44Z

@cloud-fan Submitted a PR (#13907) for converting CTAS in parquet to data source tables without hive support. Could you also review that PR? Thanks!

gatorsmile · 2016-06-29T05:38:01Z

retest this please

SparkQA · 2016-06-29T07:30:27Z

Test build #61450 has finished for PR 13886 at commit e4cc35d.

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

gatorsmile · 2016-06-30T04:56:08Z

Could you please review this PR again? @cloud-fan Thanks!

gatorsmile · 2016-07-05T18:24:58Z

retest this please

SparkQA · 2016-07-05T20:26:46Z

Test build #61771 has finished for PR 13886 at commit e4cc35d.

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

…bleAsSelect

SparkQA · 2016-07-25T19:29:28Z

Test build #62827 has finished for PR 13886 at commit 472896f.

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

gatorsmile · 2016-08-04T19:46:23Z

cc @cloud-fan This is not contained in #14482. Should I leave it open? or you will merge it into the PR?

cloud-fan · 2016-08-08T03:11:39Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala

                  s"$numStaticPartitions partition column(s) having constant value(s).")
            }

+          case c if c.getClass.getName ==


after #14482 , we can have a un-hacky way to handle it, can you update?

Sure, will fix it soon. Thanks!

…bleAsSelect

SparkQA · 2016-08-08T17:20:24Z

Test build #63365 has finished for PR 13886 at commit 8a4d2b2.

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

cloud-fan · 2016-08-09T00:25:41Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala

  def apply(plan: LogicalPlan): Unit = {
    plan.foreach {
+      case CreateTable(tableDesc, _, Some(_))
+          if tableDesc.provider.get == "hive" && sparkConf.get(CATALOG_IMPLEMENTATION) != "hive" =>


Sorry I just realized this can't be backported to 2.0.

For master, I'd like to implement it directly. Then we can think about how to deal with 2.0.

I see. That sounds reasonable to me. Do you think we can create a data source table in this specific case? If so, I can submit another PR for it.

I think we already do? The problem is APPEND mode, we need to read schema of the existing hive table.

Yeah, you are right. We did it for a few cases, but we are not able to handle all the cases. See the following code:

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala

Lines 1017 to 1030 in 5959df2

val hasStorageProperties = (ctx.createFileFormat != null) || (ctx.rowFormat != null)

if (conf.convertCTAS && !hasStorageProperties) {

// At here, both rowStorage.serdeProperties and fileStorage.serdeProperties

// are empty Maps.

val optionsWithPath = if (location.isDefined) {

Map("path" -> location.get)

} else {

Map.empty[String, String]

}

val newTableDesc = tableDesc.copy(

storage = CatalogStorageFormat.empty.copy(properties = optionsWithPath),

provider = Some(conf.defaultDataSourceName)

)

We can extend it to support more cases, like another PR: #13907. However, this sounds like we are unable to support all the cases when Hive is not enabled.

Regarding the issue you mentioned above, (reading the schema from the existing Hive table and then append to another table), there are two cases:

When Hive support is enabled, another fix of the PR [SPARK-16803] [SQL] SaveAsTable does not work when source DataFrame is built on a Hive Table #14410 is trying to resolve it. Will submit it soon.

When Hive support is not enabled, reading Hive metastore is impossible. Thus, should we still issue an exception?

Sorry, this also looks very confusing to me. Hopefully, my description is a little bit clear.

ah sorry I misread this PR. When Hive support is not enabled, we definitely need to throw exception for hive tables in CTAS.

cloud-fan · 2016-08-09T03:21:49Z

We can create a new rule for this kind of check and put it only in sql module, i.e. SessionState

…bleAsSelect

SparkQA · 2016-08-10T05:04:32Z

Test build #63495 has finished for PR 13886 at commit 79c21b6.

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

cloud-fan · 2016-08-10T06:03:23Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala

+ */
+object HiveOnlyCheck extends (LogicalPlan => Unit) {
+  def apply(plan: LogicalPlan): Unit = {
+


put it in class not in method

cloud-fan · 2016-08-10T06:06:43Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala

      "WITH SERDEPROPERTIES ('spark.sql.sources.me'='anything')")
  }

+  test("Create Cataloged Table As Select") {


hive table?

cloud-fan · 2016-08-10T06:08:41Z

Can you update the PR description? I think it should throw exception in planner before your PR.

gatorsmile · 2016-08-10T06:40:13Z

Sure, let me fix them now. Thanks!

cloud-fan · 2016-08-10T06:53:54Z

LGTM, pending jenkins, thanks for working on it!

SparkQA · 2016-08-10T08:41:06Z

Test build #63510 has finished for PR 13886 at commit 037388a.

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

viirya · 2016-08-10T08:49:54Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala

  }

+  test("Create Hive Table As Select") {
+    import testImplicits._


nit: import testImplicits._ is used in many test cases in DDLSuite. We can import it only once.

cloud-fan · 2016-08-10T09:08:39Z

thanks, merging to master!

gatorsmile added 3 commits June 24, 2016 00:08

fix

81413e0

clean test case

8947445

Better Error message.

db3d44c

fix test case

e4cc35d

Merge remote-tracking branch 'upstream/master' into createCatalogedTa…

472896f

…bleAsSelect

cloud-fan reviewed Aug 8, 2016
View reviewed changes

gatorsmile added 2 commits August 7, 2016 22:37

Merge remote-tracking branch 'upstream/master' into createCatalogedTa…

5725309

…bleAsSelect

address comments

8a4d2b2

cloud-fan reviewed Aug 9, 2016
View reviewed changes

gatorsmile added 2 commits August 9, 2016 19:32

Merge remote-tracking branch 'upstream/master' into createCatalogedTa…

a741ae2

…bleAsSelect

address comments

79c21b6

cloud-fan reviewed Aug 10, 2016
View reviewed changes

address comments

037388a

viirya reviewed Aug 10, 2016
View reviewed changes

asfgit closed this in 2b10ebe Aug 10, 2016

	val hasStorageProperties = (ctx.createFileFormat != null) \|\| (ctx.rowFormat != null)
	if (conf.convertCTAS && !hasStorageProperties) {
	// At here, both rowStorage.serdeProperties and fileStorage.serdeProperties
	// are empty Maps.
	val optionsWithPath = if (location.isDefined) {
	Map("path" -> location.get)
	} else {
	Map.empty[String, String]
	}

	val newTableDesc = tableDesc.copy(
	storage = CatalogStorageFormat.empty.copy(properties = optionsWithPath),
	provider = Some(conf.defaultDataSourceName)
	)

[SPARK-16185] [SQL] Better Error Messages When Creating Table As Select Without Enabling Hive Support #13886

[SPARK-16185] [SQL] Better Error Messages When Creating Table As Select Without Enabling Hive Support #13886

Uh oh!

Conversation

gatorsmile commented Jun 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Jun 24, 2016

Uh oh!

SparkQA commented Jun 24, 2016

Uh oh!

SparkQA commented Jun 24, 2016

Uh oh!

gatorsmile commented Jun 24, 2016

Uh oh!

cloud-fan commented Jun 25, 2016

Uh oh!

gatorsmile commented Jun 25, 2016

Uh oh!

gatorsmile commented Jun 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gatorsmile commented Jun 29, 2016

Uh oh!

SparkQA commented Jun 29, 2016

Uh oh!

gatorsmile commented Jun 30, 2016

Uh oh!

gatorsmile commented Jul 5, 2016

Uh oh!

SparkQA commented Jul 5, 2016

Uh oh!

SparkQA commented Jul 25, 2016

Uh oh!

gatorsmile commented Aug 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Aug 9, 2016

Uh oh!

SparkQA commented Aug 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Aug 10, 2016

Uh oh!

gatorsmile commented Aug 10, 2016

Uh oh!

cloud-fan commented Aug 10, 2016

Uh oh!

SparkQA commented Aug 10, 2016

Uh oh!

viirya Aug 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Aug 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

gatorsmile commented Jun 24, 2016 •

edited

Loading

gatorsmile commented Jun 26, 2016 •

edited

Loading

viirya Aug 10, 2016 •

edited

Loading