[SPARK-28313][SQL] Spark sql null type incompatible with hive void type #25085

ulysses-you · 2019-07-09T13:11:38Z

What changes were proposed in this pull request?

SPARK-20680, PR, but actually this Jira was not solved.

Spark is incompatible with hive void type. When table schema contains void type, spark throw exception in ddl option, like desc, show create table..

Also, spark catalog.createTable can create NullType Table that is not allowed.

Goal:

Support hive void column type for desc,show create table option
Add rule to throw exception when catalog.create NullType StructField, see 25198

How was this patch tested?

UT

merge master

ulysses-you · 2019-07-11T01:54:05Z

cc @cloud-fan @gatorsmile

cloud-fan · 2019-07-11T09:14:39Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

+    val expectedMsg = "DataType void is not supported"
+    withTable("t") {
+      val e = intercept[AnalysisException] {
+        sql("create table t (a void)")


how is this related to hive compatibility? It seems to me that you just change this statement from parser exception to analysis exception.

This code is just to confirm spark cannot do this ddl that compatible with hive.

cloud-fan · 2019-07-11T09:14:53Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

+    }
+
+    withTable("t") {
+      sql("CREATE TABLE t AS SELECT NULL AS col ")


what's the behavior before your patch?

Throw a SparkException 'Cannot recognize hive type string: NULL'

HyukjinKwon · 2019-07-18T07:39:49Z

ok to test

SparkQA · 2019-07-18T07:43:13Z

Test build #107837 has finished for PR 25085 at commit 64c02c7.

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

ulysses-you · 2019-07-18T10:08:34Z

cc @HyukjinKwon ok to test, please add an another test.

SparkQA · 2019-07-18T10:18:03Z

Test build #107842 has finished for PR 25085 at commit 997abb2.

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

ulysses-you · 2019-07-18T10:26:10Z

ok to test

SparkQA · 2019-07-18T12:04:20Z

Test build #107843 has finished for PR 25085 at commit d4679ad.

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

ulysses-you · 2019-07-18T12:30:49Z

ok to test

SparkQA · 2019-07-18T15:09:14Z

Test build #107852 has finished for PR 25085 at commit f3eecfb.

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

cloud-fan · 2019-07-18T15:24:43Z

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

+/**
+ * SPARK-28313: Spark sql null type incompatible with hive void type
+ */
+object CreateTableCheck extends Rule[LogicalPlan] {


Can we make a new PR for this change? This is a behavior change, which needs more discussion and needs to add something to migration guide.

Sure. As i say, this pr has two goals.

compatibility with hive

reject create table use NullType

So it's ok to make new pr for goal 2.

SparkQA · 2019-07-19T04:51:28Z

Test build #107872 has finished for PR 25085 at commit c9fc32c.

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

SparkQA · 2019-07-19T07:05:02Z

Test build #107885 has finished for PR 25085 at commit e71c4d2.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

ulysses-you · 2019-07-20T12:38:14Z

retest this please

HyukjinKwon · 2019-07-26T04:52:53Z

retest this please

SparkQA · 2019-07-26T07:05:02Z

Test build #108194 has finished for PR 25085 at commit e71c4d2.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-08-05T16:15:58Z

Retest this please.

SparkQA · 2019-08-05T18:58:18Z

Test build #108672 has finished for PR 25085 at commit e71c4d2.

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

HyukjinKwon · 2019-09-16T23:43:25Z

ok to test

SparkQA · 2019-09-17T03:49:56Z

Test build #110676 has finished for PR 25085 at commit e71c4d2.

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

cloud-fan · 2019-09-17T06:31:03Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/NullType.scala


  private[spark] override def asNullable: NullType = this
+
+  override def simpleString: String = "void"


This is also an unrelated change. I like the new naming, but it's unnecessary to the hive compatibility issue.

cloud-fan · 2019-09-17T06:32:33Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala

-      CatalystSqlParser.parseDataType(hc.getType)
+      hc.getType match {
+        // SPARK-28313 compatible hive void type
+        case "void" => NullType


I believe this is the fix we need. But I'm curious about when can this happen if hive forbids defining void type columns.

Fix this point, hive void type will be converted to null type.
Such as, t1 have a void type column c1, when execute show create table $tbl.
Spark:
create table t1 (c1 null)
Hive:
create table t1 (c1 void)

And without fix this point, spark will throw an exception.

But I'm curious about when can this happen if hive forbids defining void type columns.

Can hive do create table t1 (c1 void)?

No, hive can not but hive can do create table t1 as select null as c1.

can we make this fix surgical and only keep this change?

How about add code to check and change null to void at ShowCreateTableCommand.showCreateHiveTable() ?

Does it have to be in this PR? If it has to, I'm ok with it.

Yeah~ If you have a time, you could review the latest commit.

SparkQA · 2019-09-26T05:17:09Z

Test build #111382 has finished for PR 25085 at commit b4cc595.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2019-09-26T07:05:01Z

Test build #111391 has finished for PR 25085 at commit 9997ac1.

This patch fails due to an unknown error code, -9.
This patch does not merge cleanly.
This patch adds no public classes.

…8313

SparkQA · 2019-09-30T06:55:14Z

Test build #111587 has finished for PR 25085 at commit ec3839e.

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

SparkQA · 2019-09-30T07:05:01Z

Test build #111588 has finished for PR 25085 at commit aa08658.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

ulysses-you · 2019-10-09T01:34:04Z

@cloud-fan Do you have any time to take a look ?

github-actions · 2020-01-18T00:08:01Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

Merge pull request #1 from apache/master

b259859

merge master

dongjoon-hyun added the SQL label Jul 9, 2019

cloud-fan reviewed Jul 11, 2019

View reviewed changes

cloud-fan reviewed Jul 18, 2019

View reviewed changes

ulysses-you mentioned this pull request Jul 19, 2019

[SPARK-28443][SQL] Spark sql add exception when create field type NullType #25198

Closed

cloud-fan reviewed Sep 17, 2019

View reviewed changes

refactor code

b4cc595

ulysses-you force-pushed the SPARK-28313 branch from e71c4d2 to b4cc595 Compare September 26, 2019 03:19

fix

9997ac1

ulysses-you added 3 commits September 30, 2019 10:56

Merge branch 'master' of https://github.com/apache/spark into SPARK-2…

740dc2f

…8313

fix conflict

ec3839e

rollback HivePartitionFilteringSuite

aa08658

github-actions bot added the Stale label Jan 18, 2020

github-actions bot closed this Jan 19, 2020


		private[spark] override def asNullable: NullType = this

		override def simpleString: String = "void"

[SPARK-28313][SQL] Spark sql null type incompatible with hive void type #25085

[SPARK-28313][SQL] Spark sql null type incompatible with hive void type #25085

Uh oh!

Conversation

ulysses-you commented Jul 9, 2019 • 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!

ulysses-you commented Jul 11, 2019

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!

HyukjinKwon commented Jul 18, 2019

Uh oh!

SparkQA commented Jul 18, 2019

Uh oh!

ulysses-you commented Jul 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Jul 18, 2019

Uh oh!

ulysses-you commented Jul 18, 2019

Uh oh!

SparkQA commented Jul 18, 2019

Uh oh!

ulysses-you commented Jul 18, 2019

Uh oh!

SparkQA commented Jul 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulysses-you Jul 19, 2019 • 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!

SparkQA commented Jul 19, 2019

Uh oh!

SparkQA commented Jul 19, 2019

Uh oh!

ulysses-you commented Jul 20, 2019

Uh oh!

HyukjinKwon commented Jul 26, 2019

Uh oh!

SparkQA commented Jul 26, 2019

Uh oh!

dongjoon-hyun commented Aug 5, 2019

Uh oh!

SparkQA commented Aug 5, 2019

Uh oh!

HyukjinKwon commented Sep 16, 2019

Uh oh!

SparkQA commented Sep 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ulysses-you Sep 25, 2019 • 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!

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!

Choose a reason for hiding this comment

Uh oh!

ulysses-you commented Jul 9, 2019 •

edited

Loading

ulysses-you commented Jul 18, 2019 •

edited

Loading

ulysses-you Jul 19, 2019 •

edited

Loading

ulysses-you Sep 25, 2019 •

edited

Loading