This is an automated email from the ASF dual-hosted git repository.

maxgekk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 0f20678  [SPARK-37332][SQL] Allow ANSI intervals in `ALTER TABLE .. 
ADD COLUMNS`
0f20678 is described below

commit 0f20678fc50aaf26359d9751fe96b15dc2e12540
Author: Max Gekk <max.g...@gmail.com>
AuthorDate: Tue Nov 16 10:30:11 2021 +0300

    [SPARK-37332][SQL] Allow ANSI intervals in `ALTER TABLE .. ADD COLUMNS`
    
    ### What changes were proposed in this pull request?
    In the PR, I propose to allow ANSI intervals: year-month and day-time 
intervals in the `ALTER TABLE .. ADD COLUMNS` command for tables in v1 and v2 
In-Memory catalogs. Also added an unified test suite to migrate related tests 
in the future.
    
    ### Why are the changes needed?
    To improve user experience with Spark SQL. After the changes, users will be 
able to add columns with ANSI intervals instead of dropping and creating new 
table.
    
    ### Does this PR introduce _any_ user-facing change?
    In some sense, yes. After the changes, the command doesn't output any error 
message.
    
    ### How was this patch tested?
    By running new test suite:
    ```
    $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly 
*AlterTableAddColumnsSuite"
    $ build/sbt -Phive-2.3 "test:testOnly *HiveDDLSuite"
    ```
    
    Closes #34600 from MaxGekk/add-columns-ansi-intervals.
    
    Authored-by: Max Gekk <max.g...@gmail.com>
    Signed-off-by: Max Gekk <max.g...@gmail.com>
---
 .../sql/catalyst/analysis/CheckAnalysis.scala      |  6 +--
 .../plans/logical/v2AlterTableCommands.scala       |  2 +-
 .../apache/spark/sql/catalyst/util/TypeUtils.scala |  4 +-
 .../command/AlterTableAddColumnsSuiteBase.scala    | 53 ++++++++++++++++++++++
 .../command/v1/AlterTableAddColumnsSuite.scala     | 38 ++++++++++++++++
 .../command/v2/AlterTableAddColumnsSuite.scala     | 28 ++++++++++++
 .../spark/sql/hive/execution/HiveDDLSuite.scala    |  5 --
 .../command/AlterTableAddColumnsSuite.scala        | 46 +++++++++++++++++++
 8 files changed, 170 insertions(+), 12 deletions(-)

diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
index 1a105ad..5bf37a2 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
@@ -464,12 +464,10 @@ trait CheckAnalysis extends PredicateHelper with 
LookupCatalog {
               failAnalysis(s"Invalid partitioning: ${badReferences.mkString(", 
")}")
             }
 
-            create.tableSchema.foreach(f =>
-              TypeUtils.failWithIntervalType(f.dataType, forbidAnsiIntervals = 
false))
+            create.tableSchema.foreach(f => 
TypeUtils.failWithIntervalType(f.dataType))
 
           case write: V2WriteCommand if write.resolved =>
-            write.query.schema.foreach(f =>
-              TypeUtils.failWithIntervalType(f.dataType, forbidAnsiIntervals = 
false))
+            write.query.schema.foreach(f => 
TypeUtils.failWithIntervalType(f.dataType))
 
           case alter: AlterTableCommand =>
             checkAlterTableCommand(alter)
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
index 2eb828e..302a810 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
@@ -134,7 +134,7 @@ case class ReplaceColumns(
     table: LogicalPlan,
     columnsToAdd: Seq[QualifiedColType]) extends AlterTableCommand {
   columnsToAdd.foreach { c =>
-    TypeUtils.failWithIntervalType(c.dataType, forbidAnsiIntervals = false)
+    TypeUtils.failWithIntervalType(c.dataType)
   }
 
   override lazy val resolved: Boolean = table.resolved && 
columnsToAdd.forall(_.resolved)
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/TypeUtils.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/TypeUtils.scala
index 144508c..729a26b 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/TypeUtils.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/TypeUtils.scala
@@ -98,8 +98,8 @@ object TypeUtils {
     case _ => false
   }
 
-  def failWithIntervalType(dataType: DataType, forbidAnsiIntervals: Boolean = 
true): Unit = {
-    invokeOnceForInterval(dataType, forbidAnsiIntervals) {
+  def failWithIntervalType(dataType: DataType): Unit = {
+    invokeOnceForInterval(dataType, forbidAnsiIntervals = false) {
       throw QueryCompilationErrors.cannotUseIntervalTypeInTableSchemaError()
     }
   }
diff --git 
a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableAddColumnsSuiteBase.scala
 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableAddColumnsSuiteBase.scala
new file mode 100644
index 0000000..9a9b337
--- /dev/null
+++ 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableAddColumnsSuiteBase.scala
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command
+
+import java.time.{Duration, Period}
+
+import org.apache.spark.sql.{QueryTest, Row}
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. ADD COLUMNS` 
command that
+ * check V1 and V2 table catalogs. The tests that cannot run for all supported 
catalogs are
+ * located in more specific test suites:
+ *
+ *   - V2 table catalog tests:
+ *     `org.apache.spark.sql.execution.command.v2.AlterTableAddColumnsSuite`
+ *   - V1 table catalog tests:
+ *     
`org.apache.spark.sql.execution.command.v1.AlterTableAddColumnsSuiteBase`
+ *     - V1 In-Memory catalog:
+ *       `org.apache.spark.sql.execution.command.v1.AlterTableAddColumnsSuite`
+ *     - V1 Hive External catalog:
+ *       
`org.apache.spark.sql.hive.execution.command.AlterTableAddColumnsSuite`
+ */
+trait AlterTableAddColumnsSuiteBase extends QueryTest with DDLCommandTestUtils 
{
+  override val command = "ALTER TABLE .. ADD COLUMNS"
+
+  test("add an ANSI interval columns") {
+    assume(!catalogVersion.contains("Hive")) // Hive catalog doesn't support 
the interval types
+
+    withNamespaceAndTable("ns", "tbl") { t =>
+      sql(s"CREATE TABLE $t (id bigint) $defaultUsing")
+      sql(s"ALTER TABLE $t ADD COLUMNS (ym INTERVAL YEAR, dt INTERVAL HOUR)")
+      sql(s"INSERT INTO $t SELECT 0, INTERVAL '100' YEAR, INTERVAL '10' HOUR")
+      checkAnswer(
+        sql(s"SELECT id, ym, dt data FROM $t"),
+        Seq(Row(0, Period.ofYears(100), Duration.ofHours(10))))
+    }
+  }
+}
diff --git 
a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableAddColumnsSuite.scala
 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableAddColumnsSuite.scala
new file mode 100644
index 0000000..bca478c
--- /dev/null
+++ 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableAddColumnsSuite.scala
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v1
+
+import org.apache.spark.sql.execution.command
+
+/**
+ * This base suite contains unified tests for the `ALTER TABLE .. ADD COLUMNS` 
command that
+ * check V1 table catalogs. The tests that cannot run for all V1 catalogs are 
located in more
+ * specific test suites:
+ *
+ *   - V1 In-Memory catalog:
+ *     `org.apache.spark.sql.execution.command.v1.AlterTableAddColumnsSuite`
+ *   - V1 Hive External catalog:
+ *     `org.apache.spark.sql.hive.execution.command.AlterTableAddColumnsSuite`
+ */
+trait AlterTableAddColumnsSuiteBase extends 
command.AlterTableAddColumnsSuiteBase
+
+/**
+ * The class contains tests for the `ALTER TABLE .. ADD COLUMNS` command to 
check
+ * V1 In-Memory table catalog.
+ */
+class AlterTableAddColumnsSuite extends AlterTableAddColumnsSuiteBase with 
CommandSuiteBase
diff --git 
a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterTableAddColumnsSuite.scala
 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterTableAddColumnsSuite.scala
new file mode 100644
index 0000000..7a548c8
--- /dev/null
+++ 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterTableAddColumnsSuite.scala
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command.v2
+
+import org.apache.spark.sql.execution.command
+
+/**
+ * The class contains tests for the `ALTER TABLE .. ADD COLUMNS` command
+ * to check V2 table catalogs.
+ */
+class AlterTableAddColumnsSuite
+  extends command.AlterTableAddColumnsSuiteBase
+  with CommandSuiteBase
diff --git 
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
 
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
index 4746f0a..3bf3ee5 100644
--- 
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
+++ 
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
@@ -3018,11 +3018,6 @@ class HiveDDLSuite
         }.getMessage
         assert(errMsg.contains(s"Hive table `default`.`$tbl` with ANSI 
intervals is not supported"))
       }
-      sql(s"CREATE TABLE $tbl STORED AS PARQUET AS SELECT 1")
-      val errMsg2 = intercept[ParseException] {
-        sql(s"ALTER TABLE $tbl ADD COLUMNS (ym INTERVAL YEAR)")
-      }.getMessage
-      assert(errMsg2.contains("Cannot use interval type in the table schema"))
     }
   }
 
diff --git 
a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/AlterTableAddColumnsSuite.scala
 
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/AlterTableAddColumnsSuite.scala
new file mode 100644
index 0000000..2b28890
--- /dev/null
+++ 
b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/AlterTableAddColumnsSuite.scala
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.hive.execution.command
+
+import org.apache.spark.sql.execution.command.v1
+
+/**
+ * The class contains tests for the `ALTER TABLE .. ADD COLUMNS` command to 
check
+ * V1 Hive external table catalog.
+ */
+class AlterTableAddColumnsSuite
+  extends v1.AlterTableAddColumnsSuiteBase
+  with CommandSuiteBase {
+
+  test("SPARK-36949: Disallow tables with ANSI intervals when the provider is 
Hive") {
+    def check(tbl: String): Unit = {
+      val errMsg = intercept[UnsupportedOperationException] {
+        sql(s"ALTER TABLE $tbl ADD COLUMNS (ym INTERVAL YEAR)")
+      }.getMessage
+      assert(errMsg.contains("ANSI intervals is not supported"))
+    }
+    withNamespaceAndTable("ns", "tbl") { tbl =>
+      sql(s"CREATE TABLE $tbl (id INT) $defaultUsing")
+      check(tbl)
+    }
+    withNamespaceAndTable("ns", "tbl") { tbl =>
+      sql(s"CREATE TABLE $tbl STORED AS PARQUET AS SELECT 1")
+      check(tbl)
+    }
+  }
+}

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to