From 05b57bc1ebe6ebec51b25f001a7c8491cb4c7edf Mon Sep 17 00:00:00 2001 From: Jia Fan Date: Fri, 8 Sep 2023 13:17:55 -0700 Subject: [PATCH] [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 Signed-off-by: Dongjoon Hyun (cherry picked from commit 4dd4737b34b1215e374674b777c2eb8906a29ed7) Signed-off-by: Dongjoon Hyun --- .../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 609cfab2d568e..ebecb6f507e6a 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 String[] fieldNames() { /** * 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 eb9d45f06ec79..b02c4fac12dec 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 a18c767570f01..ca60e3212e686 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) {