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

dongjoon 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 4dd4737b34b [SPARK-45075][SQL] Fix alter table with invalid default 
value will not report error
4dd4737b34b is described below

commit 4dd4737b34b1215e374674b777c2eb8906a29ed7
Author: Jia Fan <fanjiaemi...@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 <fanjiaemi...@qq.com>
    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-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to