You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by do...@apache.org on 2023/09/08 20:18:15 UTC

[spark] branch branch-3.5 updated: [SPARK-45075][SQL] Fix alter table with invalid default value will not report error

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

dongjoon pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.5 by this push:
     new 05b57bc1ebe [SPARK-45075][SQL] Fix alter table with invalid default value will not report error
05b57bc1ebe is described below

commit 05b57bc1ebe6ebec51b25f001a7c8491cb4c7edf
Author: Jia Fan <fa...@qq.com>
AuthorDate: Fri Sep 8 13:17:55 2023 -0700

    [SPARK-45075][SQL] Fix alter table with invalid default value will not report error
    
    ### What changes were proposed in this pull request?
    This PR make sure ALTER TABLE ALTER COLUMN with invalid default value on DataSource V2 will report error, before this PR it will alter sucess.
    
    ### Why are the changes needed?
    Fix the error behavior on DataSource V2 with ALTER TABLE statement.
    
    ### Does this PR introduce _any_ user-facing change?
    Yes, the invalid default value will report error.
    
    ### How was this patch tested?
    Add new test.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    No.
    
    Closes #42810 from Hisoka-X/SPARK-45075_alter_invalid_default_value_on_v2.
    
    Authored-by: Jia Fan <fa...@qq.com>
    Signed-off-by: Dongjoon Hyun <dh...@apple.com>
    (cherry picked from commit 4dd4737b34b1215e374674b777c2eb8906a29ed7)
    Signed-off-by: Dongjoon Hyun <dh...@apple.com>
---
 .../spark/sql/connector/catalog/TableChange.java   |  3 +--
 .../plans/logical/v2AlterTableCommands.scala       | 11 ++++++--
 .../spark/sql/connector/AlterTableTests.scala      | 29 ++++++++++++++++++++++
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableChange.java b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableChange.java
index 609cfab2d56..ebecb6f507e 100644
--- a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableChange.java
+++ b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableChange.java
@@ -696,9 +696,8 @@ public interface TableChange {
     /**
      * Returns the column default value SQL string (Spark SQL dialect). The default value literal
      * is not provided as updating column default values does not need to back-fill existing data.
-     * Null means dropping the column default value.
+     * Empty string means dropping the column default value.
      */
-    @Nullable
     public String newDefaultValue() { return newDefaultValue; }
 
     @Override
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 eb9d45f06ec..b02c4fac12d 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
@@ -17,9 +17,9 @@
 
 package org.apache.spark.sql.catalyst.plans.logical
 
-import org.apache.spark.sql.catalyst.analysis.{FieldName, FieldPosition}
+import org.apache.spark.sql.catalyst.analysis.{FieldName, FieldPosition, ResolvedFieldName}
 import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
-import org.apache.spark.sql.catalyst.util.TypeUtils
+import org.apache.spark.sql.catalyst.util.{ResolveDefaultColumns, TypeUtils}
 import org.apache.spark.sql.connector.catalog.{TableCatalog, TableChange}
 import org.apache.spark.sql.errors.QueryCompilationErrors
 import org.apache.spark.sql.types.DataType
@@ -228,6 +228,13 @@ case class AlterColumn(
       TableChange.updateColumnPosition(colName, newPosition.position)
     }
     val defaultValueChange = setDefaultExpression.map { newDefaultExpression =>
+      if (newDefaultExpression.nonEmpty) {
+        // SPARK-45075: We call 'ResolveDefaultColumns.analyze' here to make sure that the default
+        // value parses successfully, and return an error otherwise
+        val newDataType = dataType.getOrElse(column.asInstanceOf[ResolvedFieldName].field.dataType)
+        ResolveDefaultColumns.analyze(column.name.last, newDataType, newDefaultExpression,
+          "ALTER TABLE ALTER COLUMN")
+      }
       TableChange.updateColumnDefaultValue(colName, newDefaultExpression)
     }
     typeChange.toSeq ++ nullabilityChange ++ commentChange ++ positionChange ++ defaultValueChange
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/connector/AlterTableTests.scala b/sql/core/src/test/scala/org/apache/spark/sql/connector/AlterTableTests.scala
index a18c767570f..ca60e3212e6 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/connector/AlterTableTests.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/connector/AlterTableTests.scala
@@ -363,6 +363,35 @@ trait AlterTableTests extends SharedSparkSession with QueryErrorsBase {
     }
   }
 
+  test("SPARK-45075: ALTER COLUMN with invalid default value") {
+    withSQLConf(SQLConf.DEFAULT_COLUMN_ALLOWED_PROVIDERS.key -> s"$v2Format, ") {
+      withTable("t") {
+        sql(s"create table t(i boolean) using $v2Format")
+        // The default value fails to analyze.
+        checkError(
+          exception = intercept[AnalysisException] {
+            sql("alter table t add column s bigint default badvalue")
+          },
+          errorClass = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION",
+          parameters = Map(
+            "statement" -> "ALTER TABLE",
+            "colName" -> "`s`",
+            "defaultValue" -> "badvalue"))
+
+        sql("alter table t add column s bigint default 3L")
+        checkError(
+          exception = intercept[AnalysisException] {
+            sql("alter table t alter column s set default badvalue")
+          },
+          errorClass = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION",
+          parameters = Map(
+            "statement" -> "ALTER TABLE ALTER COLUMN",
+            "colName" -> "`s`",
+            "defaultValue" -> "badvalue"))
+      }
+    }
+  }
+
   test("AlterTable: add complex column") {
     val t = fullTableName("table_name")
     withTable(t) {


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