[SPARK-36949][SQL] Disallow Hive provider tables with ANSI intervals #34259

MaxGekk · 2021-10-12T11:04:10Z

What changes were proposed in this pull request?

In the PR, I propose to check column types of tables that use Hive as the provider, and disallow tables with ANSI intervals. Currently, Hive Metastore & SerDe don't support creating tables with interval types. So, even Spark converts Catalyst's ANSI interval types to Hive's types:

YearMonthIntervalType -> INTERVAL_YEAR_MONTH
DayTimeIntervalType -> INTERVAL_DAY_TIME

Hive Metastore fails with the error (for parquet SerDe):

Caused by: java.lang.UnsupportedOperationException: Unknown field type: interval_year_month
	at org.apache.hadoop.hive.ql.io.parquet.serde.ArrayWritableObjectInspector.getObjectInspector(ArrayWritableObjectInspector.java:141)
	at org.apache.hadoop.hive.ql.io.parquet.serde.ArrayWritableObjectInspector.<init>(ArrayWritableObjectInspector.java:86)
	at org.apache.hadoop.hive.ql.io.parquet.serde.ArrayWritableObjectInspector.<init>(ArrayWritableObjectInspector.java:59)
	at org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe.initialize(ParquetHiveSerDe.java:129)

at https://github.com/apache/hive/blame/master/ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ArrayWritableObjectInspector.java#L94 .

After the changes, Spark will verify column types before sending unnecessary requests to Hive Metastore.

I more details, I added new check to the existing function DDLUtils.checkDataColNames(), and renamed the function to checkTableColumns() because it checks table column names together with column types.

Note: We still allow tables of non-Hive provider with ANSI intervals using Hive external catalog, see #34215.

Why are the changes needed?

To make the error message more clear and independent from Hive Metastore.

Does this PR introduce any user-facing change?

Yes. This PR changes the error message.

Before:

spark-sql> CREATE TABLE TBL STORED AS PARQUET AS SELECT INTERVAL '1-1' YEAR TO MONTH AS YM;
Error in query: org.apache.hadoop.hive.ql.metadata.HiveException: java.lang.IllegalArgumentException: Error: type expected at the position 0 of 'interval year to month' but 'interval year to month' is found.

After:

spark-sql> CREATE TABLE TBL STORED AS PARQUET AS SELECT INTERVAL '1-1' YEAR TO MONTH AS YM;
21/10/12 16:51:14 ERROR SparkSQLDriver: Failed in [CREATE TABLE TBL STORED AS PARQUET AS SELECT INTERVAL '1-1' YEAR TO MONTH AS YM]
java.lang.UnsupportedOperationException: Hive table `default`.`TBL` with ANSI intervals is not supported
	at org.apache.spark.sql.errors.QueryExecutionErrors$.hiveTableWithAnsiIntervalsError(QueryExecutionErrors.scala:1831)

How was this patch tested?

By running new tests:

$ build/sbt -Phive-2.3 "test:testOnly *HiveDDLSuite"

SparkQA · 2021-10-12T12:01:19Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48624/

SparkQA · 2021-10-12T13:01:47Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48624/

MaxGekk · 2021-10-12T14:22:35Z

@sunchao Could you take a look at this, please.

SparkQA · 2021-10-12T16:02:03Z

Test build #144147 has finished for PR 34259 at commit 9a1c538.

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

sunchao

LGTM. This also handles ALTER TABLE ADD COLUMN as well right? it's great to also add a test for that.

cloud-fan · 2021-10-12T16:45:36Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSuite.scala

+        sql(
+          s"""
+             |CREATE TABLE $tbl
+             |STORED AS PARQUET


can we also test CREATE TABLE without AS SELECT?

Since Serde does not matter, we can put the test in SQLQuerySuite under sql/hive with parquet serde only.

@cloud-fan Could you give me an example, please. The PR added a test already #34215

if INSERT works, I'm wondering why CTAS does not

The issue is in Hive's SerDe/Metastore. So, when you insert ANSI intervals to a table where Hive SerDe is not involved (provider is parquet, for instance), and we store schema in Spark's specific format to Hive external catalog, I wonder why do you wonder that INSERT works well?

In that case, we use Hive MetaStore as a store for our schema only. HMS is not aware of our types, right?

we can put the test in SQLQuerySuite under sql/hive with parquet serde only.

Why not to HiveDDLSuite, for instance? This PR is mostly about creating/modifying a table (data definition) but not about querying.

HiveDDLSuite sounds better!

MaxGekk · 2021-10-12T16:47:51Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala

+          if (schema.exists(_.dataType.isInstanceOf[AnsiIntervalType])) {
+            throw hiveTableWithAnsiIntervalsError(table.identifier.toString)
+          } else if (serde == HiveSerDe.sourceToSerDe("orc").get.serde) {


I have released that I have some concerns about the place of the check. This function is supposed to check column names not column types.

I think of new private function checkDataColTypes or checkColumnTypes, WDYT? And call the function from the same places as checkDataColNames() is called.

Renamed to the private methods to checkTableColumns

sunchao · 2021-10-12T17:20:54Z

I also wonder if ANSI intervals will work for Parquet data source tables if spark.sql.hive.convertMetastoreParquet is set to false.

Edit: NVM, I see this config only affect Hive tables.

SparkQA · 2021-10-13T10:57:45Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48677/

SparkQA · 2021-10-13T11:40:05Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48677/

SparkQA · 2021-10-13T14:23:04Z

Test build #144198 has finished for PR 34259 at commit 88cc112.

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

cloud-fan · 2021-10-13T15:01:20Z

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

+          s"""
+             |CREATE TABLE $tbl
+             |STORED AS ORC
+             |AS SELECT


can we also test CREATE TABLE without SELECT?

SparkQA · 2021-10-13T16:07:00Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48683/

MaxGekk · 2021-10-13T16:42:02Z

Merging to master. Thank you, @sunchao and @cloud-fan for review.

SparkQA · 2021-10-13T17:07:04Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48683/

SparkQA · 2021-10-13T20:05:34Z

Test build #144204 has finished for PR 34259 at commit a855322.

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

Disallow Hive provider tables with ANSI intervals

9a1c538

github-actions bot added the SQL label Oct 12, 2021

MaxGekk changed the title ~~[WIP][SPARK-36949][SQL] Disallow Hive provider tables with ANSI intervals~~ [SPARK-36949][SQL] Disallow Hive provider tables with ANSI intervals Oct 12, 2021

MaxGekk marked this pull request as ready for review October 12, 2021 14:11

MaxGekk requested review from cloud-fan and sarutak October 12, 2021 14:11

sunchao approved these changes Oct 12, 2021

View reviewed changes

cloud-fan reviewed Oct 12, 2021

View reviewed changes

MaxGekk commented Oct 12, 2021

View reviewed changes

MaxGekk added 2 commits October 13, 2021 11:45

checkDataColNames -> checkTableColumns

2879ab7

Move the tests to HiveDDLSuite

88cc112

cloud-fan reviewed Oct 13, 2021

View reviewed changes

cloud-fan approved these changes Oct 13, 2021

View reviewed changes

Addressing Wenchen's review comment

a855322

MaxGekk closed this in 1aa3611 Oct 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-36949][SQL] Disallow Hive provider tables with ANSI intervals #34259

[SPARK-36949][SQL] Disallow Hive provider tables with ANSI intervals #34259

MaxGekk commented Oct 12, 2021 •

edited

Loading

SparkQA commented Oct 12, 2021

SparkQA commented Oct 12, 2021

MaxGekk commented Oct 12, 2021

SparkQA commented Oct 12, 2021

sunchao left a comment

cloud-fan Oct 12, 2021

cloud-fan Oct 12, 2021

MaxGekk Oct 12, 2021 •

edited

Loading

cloud-fan Oct 12, 2021

MaxGekk Oct 12, 2021 •

edited

Loading

MaxGekk Oct 13, 2021

cloud-fan Oct 13, 2021

MaxGekk Oct 12, 2021 •

edited

Loading

MaxGekk Oct 12, 2021

MaxGekk Oct 13, 2021

sunchao commented Oct 12, 2021 •

edited

Loading

SparkQA commented Oct 13, 2021

SparkQA commented Oct 13, 2021

SparkQA commented Oct 13, 2021

cloud-fan Oct 13, 2021

SparkQA commented Oct 13, 2021

MaxGekk commented Oct 13, 2021

SparkQA commented Oct 13, 2021

SparkQA commented Oct 13, 2021

[SPARK-36949][SQL] Disallow Hive provider tables with ANSI intervals #34259

[SPARK-36949][SQL] Disallow Hive provider tables with ANSI intervals #34259

Conversation

MaxGekk commented Oct 12, 2021 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Oct 12, 2021

SparkQA commented Oct 12, 2021

MaxGekk commented Oct 12, 2021

SparkQA commented Oct 12, 2021

sunchao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxGekk Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxGekk Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxGekk Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunchao commented Oct 12, 2021 • edited Loading

SparkQA commented Oct 13, 2021

SparkQA commented Oct 13, 2021

SparkQA commented Oct 13, 2021

Choose a reason for hiding this comment

SparkQA commented Oct 13, 2021

MaxGekk commented Oct 13, 2021

SparkQA commented Oct 13, 2021

SparkQA commented Oct 13, 2021

MaxGekk commented Oct 12, 2021 •

edited

Loading

MaxGekk Oct 12, 2021 •

edited

Loading

MaxGekk Oct 12, 2021 •

edited

Loading

MaxGekk Oct 12, 2021 •

edited

Loading

sunchao commented Oct 12, 2021 •

edited

Loading