Skip to content

Commit f53ecec

Browse files
committed
Review comments
1 parent dbd29ee commit f53ecec

File tree

6 files changed

+34
-6
lines changed

6 files changed

+34
-6
lines changed

sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableChange.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ public String newDefaultValue() {
753753
/**
754754
* Returns the column default value as {@link DefaultValue}. The default value literal
755755
* is not provided as updating column default values does not need to back-fill existing data.
756-
* Empty string means dropping the column default value.
756+
* Empty string and Null literal means dropping the column default value.
757757
*/
758758
public DefaultValue newModelDefaultValue() { return newDefaultValue; }
759759

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5247,7 +5247,7 @@ class AstBuilder extends DataTypeAstBuilder
52475247
if (action.defaultExpression != null) {
52485248
Option(action.defaultExpression()).map(visitDefaultExpression)
52495249
} else if (action.dropDefault != null) {
5250-
Some(DefaultValueExpression(Literal(""), ""))
5250+
Some(DefaultValueExpression(Literal(null), ""))
52515251
} else {
52525252
None
52535253
}

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/ColumnDefinition.scala

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,12 @@ object ColumnDefinition {
193193
throw QueryCompilationErrors.defaultValuesUnresolvedExprError(
194194
"ALTER TABLE ALTER COLUMN", c.column.name.quoted, d.originalSQL, null)
195195
}
196-
validateDefaultValueExpr(d, "ALTER TABLE", c.column.name.quoted, d.dataType)
196+
validateDefaultValueExpr(d, "ALTER TABLE ALTER COLUMN",
197+
c.column.name.quoted, d.dataType)
198+
if (!d.deterministic) {
199+
throw QueryCompilationErrors.defaultValueNonDeterministicError(
200+
"ALTER TABLE ALTER COLUMN", c.column.name.quoted, d.originalSQL)
201+
}
197202
}
198203
}
199204

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2773,7 +2773,7 @@ class DDLParserSuite extends AnalysisTest {
27732773
None,
27742774
None,
27752775
None,
2776-
Some(DefaultValueExpression(Literal(""), ""))))))
2776+
Some(DefaultValueExpression(Literal(null), ""))))))
27772777
// Make sure that the parser returns an exception when the feature is disabled.
27782778
withSQLConf(SQLConf.ENABLE_DEFAULT_COLUMNS.key -> "false") {
27792779
val sql = "CREATE TABLE my_tab(a INT, b STRING NOT NULL DEFAULT \"abc\") USING parquet"

sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2DataFrameSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import org.apache.spark.sql.execution.{QueryExecution, SparkPlan}
3030
import org.apache.spark.sql.execution.ExplainUtils.stripAQEPlan
3131
import org.apache.spark.sql.execution.datasources.v2.{AlterTableExec, CreateTableExec, DataSourceV2Relation, ReplaceTableExec}
3232
import org.apache.spark.sql.internal.SQLConf
33-
import org.apache.spark.sql.types.{BooleanType, CalendarIntervalType, IntegerType, StringType}
33+
import org.apache.spark.sql.types.{BooleanType, CalendarIntervalType, IntegerType, NullType, StringType}
3434
import org.apache.spark.sql.util.QueryExecutionListener
3535
import org.apache.spark.unsafe.types.UTF8String
3636

@@ -557,7 +557,7 @@ class DataSourceV2DataFrameSuite
557557
case u: UpdateColumnDefaultValue => u
558558
}.head,
559559
new DefaultValue(
560-
"", LiteralValue(UTF8String.fromString(""), StringType)))
560+
"", LiteralValue(null, NullType)))
561561
}
562562
}
563563

sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3606,6 +3606,29 @@ class DataSourceV2SQLSuiteV1Filter
36063606
}
36073607
}
36083608

3609+
test("SPARK-52116: alter column with default value which is not deterministic") {
3610+
val foldableExpressions = Seq("1", "2 + 1")
3611+
withSQLConf(SQLConf.DEFAULT_COLUMN_ALLOWED_PROVIDERS.key -> v2Source) {
3612+
withTable("tab") {
3613+
spark.sql(s"CREATE TABLE tab (col1 DOUBLE DEFAULT 0) USING $v2Source")
3614+
val exception = analysisException(
3615+
// Rand function is not deterministic
3616+
s"ALTER TABLE tab ALTER COLUMN col1 SET DEFAULT rand()")
3617+
assert(exception.getSqlState == "42623")
3618+
assert(exception.errorClass.get == "INVALID_DEFAULT_VALUE.NON_DETERMINISTIC")
3619+
assert(exception.messageParameters("statement") == "ALTER TABLE ALTER COLUMN")
3620+
assert(exception.messageParameters("colName") == "`col1`")
3621+
assert(exception.messageParameters("defaultValue") == "rand()")
3622+
}
3623+
foldableExpressions.foreach(expr => {
3624+
withTable("tab") {
3625+
spark.sql(s"CREATE TABLE tab (col1 INT DEFAULT 100) USING $v2Source")
3626+
spark.sql(s"ALTER TABLE tab ADD COLUMN col2 DOUBLE DEFAULT $expr")
3627+
}
3628+
})
3629+
}
3630+
}
3631+
36093632
test("SPARK-49099: Switch current schema with custom spark_catalog") {
36103633
// Reset CatalogManager to clear the materialized `spark_catalog` instance, so that we can
36113634
// configure a new implementation.

0 commit comments

Comments
 (0)