[SPARK-20680][SQL] Adding HiveVoidType in Spark to be compatible with Hive #28935

LantaoJin · 2020-06-28T02:29:56Z

What changes were proposed in this pull request?

This is to address the close one #17953, as a refactor for #28833
Adding HiveVoidType, like HiveStringType, to prevent from exception when describe tables/views which the schema contain Hive VOID/NULL type.

Why are the changes needed?

Spark is incompatible with hive void type. When Hive table schema contains void type, DESC table will throw an exception in Spark.

hive> create table bad as select 1 x, null z from dual;
hive> describe bad;
OK
x int
z void

In Spark2.0.x, the behaviour to read this view is normal:

spark-sql> describe bad;
x int NULL
z void NULL
Time taken: 4.431 seconds, Fetched 2 row(s)

But in lastest Spark version, it failed with SparkException: Cannot recognize hive type string: void

spark-sql> describe bad;
17/05/09 03:12:08 ERROR thriftserver.SparkSQLDriver: Failed in [describe bad]
org.apache.spark.SparkException: Cannot recognize hive type string: void
Caused by: org.apache.spark.sql.catalyst.parser.ParseException:
DataType void() is not supported.(line 1, pos 0)
== SQL ==
void
^^^
... 61 more
org.apache.spark.SparkException: Cannot recognize hive type string: void

Does this PR introduce any user-facing change?

No

How was this patch tested?

Add unit tests

Also can manual tests

spark-sql> describe bad;
x int NULL
z null NULL
Time taken: 0.486 seconds, Fetched 2 row(s)

… Hive

HyukjinKwon

We should also fix

spark/python/pyspark/sql/tests/test_types.py

Line 486 in 68d7edf

if t != NullType:

and add null type at

spark/python/pyspark/sql/types.py

Line 752 in d21aab4

_all_atomic_types = dict((t.typeName(), t) for t in _atomic_types)

I am okay if you're not used to Python side - I can do it in a followup.

LantaoJin · 2020-06-28T02:46:54Z

Thanks @HyukjinKwon, if this could be merged, can you help on python side?

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

dongjoon-hyun · 2020-06-28T03:32:41Z

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

+ * and should NOT be used anywhere else. Any instance of these data types should be
+ * replaced by a [[NullType]] before analysis.
+ */
+class HiveNullType private() extends DataType {


I know the context, but can we name this HiveVoidType literally?

Currently, the description is interpreted like "hive null type should be replaced by a NullType before analysis".

null is a value and Hive exposes void as a type.

null is a value and Hive exposes void as a type.

You are right.

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

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala

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

dongjoon-hyun · 2020-06-28T03:44:56Z

Thank you for working on this, @LantaoJin !

SparkQA · 2020-06-28T07:05:02Z

Test build #124583 has finished for PR 28935 at commit 17b1853.

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

SparkQA · 2020-06-28T07:05:02Z

Test build #124584 has finished for PR 28935 at commit ba2ef06.

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

SparkQA · 2020-06-28T07:05:02Z

Test build #124590 has finished for PR 28935 at commit 17aace2.

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

LantaoJin · 2020-06-28T07:23:13Z

retest this please

SparkQA · 2020-06-28T12:29:13Z

Test build #124597 has finished for PR 28935 at commit 17aace2.

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

dongjoon-hyun · 2020-06-28T16:06:23Z

Thank you for updating, @LantaoJin .

dongjoon-hyun · 2020-06-28T18:01:33Z

@LantaoJin . Is there a reason why you use $ at the Scala file name?

sql/catalyst/src/main/scala/org/apache/spark/sql/types/HiveVoidType$.scala

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

LantaoJin · 2020-06-29T00:18:03Z

@LantaoJin . Is there a reason why you use $ at the Scala file name?

sql/catalyst/src/main/scala/org/apache/spark/sql/types/HiveVoidType$.scala

No, just typo. Fixed.

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

cloud-fan · 2020-06-29T04:53:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

      case ("decimal" | "dec" | "numeric", precision :: scale :: Nil) =>
        DecimalType(precision.getText.toInt, scale.getText.toInt)
      case ("interval", Nil) => CalendarIntervalType
+      case ("void", Nil) => HiveVoidType


We can just reuse NullType. We should forbid creating table with null type completely, including spark.catalog.createTable.

SparkQA · 2020-06-29T06:23:25Z

Test build #124614 has finished for PR 28935 at commit a3a1cef.

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

SparkQA · 2020-06-29T08:02:50Z

Test build #124639 has started for PR 28935 at commit 9b9d021.

cloud-fan · 2020-06-29T11:22:12Z

I think we need two changes:

forbid creating tables with void column type. This could be done in the rule CommandCheck. We can test it with spark.catalog.createTable
support "void" in the parser to fix SPARK-20680. This is only for legacy hive tables, as new tables can't have void type columns.

These two changes can be done with 2 PRs.

LantaoJin · 2020-06-29T12:33:51Z

Yes. This PR is for the second change.

cloud-fan · 2020-06-29T13:19:45Z

we need to do 1 first, otherwise it makes users be able to create tables with void type via CREATE TABLE command, while it was not possible before as the parser doesn't support it.

LantaoJin · 2020-06-29T15:10:09Z

Ah, now I understood the context. The 5th commit should be reverted in this PR, otherwise the UT will fail. And we need to do 1 first. I will work on that tomorrow. My laptop is out of power now.

LantaoJin · 2020-06-30T01:47:41Z

@cloud-fan I refactor some codes, now I think this PR could be no dependency.

cloud-fan · 2020-06-30T07:00:10Z

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala

          }
-          // Add Hive type string to metadata.
-          val cleanedDataType = HiveStringType.replaceCharType(dataType)
+          // Add Hive type 'string' and 'void' to metadata.


we can be more aggressive here: forbid void type in all cases, including hive tables.

cloud-fan · 2020-06-30T07:01:11Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

   */
  private def visitSparkDataType(ctx: DataTypeContext): DataType = {
-    HiveStringType.replaceCharType(typedVisit(ctx))
+    HiveVoidType.replaceVoidType(HiveStringType.replaceCharType(typedVisit(ctx)))


I don't get it why we need HiveVoidType. What happens if we just parse void to NullType?

Because that could indicate VOID is a Hive type, the handle processing is more unified. Or, we can just use the PR #28833

For example, below function will point the failure is due to the legacy hive void type. If we mix VOID and NULL, I am not sure it would be better than separation.

def failVoidType(dt: DataType): Unit = { if (HiveVoidType.containsVoidType(dt)) { throw new AnalysisException( "Cannot create tables with Hive VOID type.") } }

VOID and NULL are indeed the same type. We can just check null type and fail with error message: Cannot create tables with VOID type

The point is consistency: The VOID type in SQL statement should be the same as NullType specified by Scala API in spark.catalog.createTable.

@cloud-fan , ok. I will follow your suggestion to fix it in #28833 , since this PR is a refator with new type HiveVoidType. Now we don't need it.

SparkQA · 2020-06-30T22:59:03Z

Test build #124657 has finished for PR 28935 at commit 3fa76cf.

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

LantaoJin · 2020-07-08T03:45:57Z

Close this since #28833 merged. Thank you!

[SPARK-20680][SQL] Adding HiveNullType in Spark to be compatible with…

17b1853

… Hive

probot-autolabeler bot added the SQL label Jun 28, 2020

LantaoJin mentioned this pull request Jun 28, 2020

[SPARK-20680][SQL] Spark-sql do not support for creating table with void column datatype #28833

Closed

HyukjinKwon reviewed Jun 28, 2020

View reviewed changes

remove useless import

ba2ef06