You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2025/06/04 17:32:24 UTC
(spark) branch branch-4.0 updated: [SPARK-52398][SQL] Change `ALTER TABLE ALTER COLUMN TYPE STRING` not to apply default collation if original data type was instance of `StringType`
This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new 40f46fcf3038 [SPARK-52398][SQL] Change `ALTER TABLE ALTER COLUMN TYPE STRING` not to apply default collation if original data type was instance of `StringType`
40f46fcf3038 is described below
commit 40f46fcf303859beded5f7bd485a46ee7dc0e229
Author: ilicmarkodb <ma...@databricks.com>
AuthorDate: Wed Jun 4 10:32:11 2025 -0700
[SPARK-52398][SQL] Change `ALTER TABLE ALTER COLUMN TYPE STRING` not to apply default collation if original data type was instance of `StringType`
### What changes were proposed in this pull request?
Changed `ALTER TABLE ALTER COLUMN TYPE STRING` not to apply default collation if original data type was instance of `StringType`.
```
CREATE TABLE T (C1 CHAR/VARCHAR);
ALTER TABLE T DEFAULT COLLATION UTF8_LCASE;
ALTER TABLE T ALTER COLUMN C1 TYPE STRING COLLATE UTF8_LCASE;
-----------------------------------------------------------------------------------
CREATE TABLE T (C1 STRING [COLLATE XYZ])
ALTER TABLE T DEFAULT COLLATION UTF8_LCASE
ALTER TABLE T ALTER COLUMN C1 TYPE STRING // C1 -> STRING [COLLATE XYZ]
```
### Why are the changes needed?
Bug fix.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Tests added to `DefaultCollationTestSuite`.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #51085 from ilicmarkodb/branch-4.0.
Authored-by: ilicmarkodb <ma...@databricks.com>
Signed-off-by: Wenchen Fan <we...@databricks.com>
---
.../ApplyDefaultCollationToStringType.scala | 86 ++++++++++++++++++++--
.../sql/collation/DefaultCollationTestSuite.scala | 58 ++++++++++++++-
2 files changed, 135 insertions(+), 9 deletions(-)
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollationToStringType.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollationToStringType.scala
index 0082c65ab8b2..1498499e278b 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollationToStringType.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollationToStringType.scala
@@ -20,8 +20,12 @@ package org.apache.spark.sql.catalyst.analysis
import org.apache.spark.sql.catalyst.expressions.{Cast, DefaultStringProducingExpression, Expression, Literal, SubqueryExpression}
import org.apache.spark.sql.catalyst.plans.logical.{AddColumns, AlterColumns, AlterColumnSpec, AlterViewAs, ColumnDefinition, CreateTable, CreateTempView, CreateView, LogicalPlan, QualifiedColType, ReplaceColumns, ReplaceTable, V2CreateTablePlan}
import org.apache.spark.sql.catalyst.rules.Rule
-import org.apache.spark.sql.connector.catalog.TableCatalog
-import org.apache.spark.sql.types.{DataType, StringType}
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
+import org.apache.spark.sql.catalyst.util.CharVarcharUtils.CHAR_VARCHAR_TYPE_STRING_METADATA_KEY
+import org.apache.spark.sql.connector.catalog.{CatalogV2Util, Table, TableCatalog}
+import org.apache.spark.sql.errors.DataTypeErrors.toSQLId
+import org.apache.spark.sql.errors.QueryCompilationErrors
+import org.apache.spark.sql.types.{CharType, DataType, StringType, StructField, VarcharType}
/**
* Resolves string types in logical plans by assigning them the appropriate collation. The
@@ -32,10 +36,12 @@ import org.apache.spark.sql.types.{DataType, StringType}
*/
object ApplyDefaultCollationToStringType extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
+ val preprocessedPlan = pruneRedundantAlterColumnTypes(plan)
+
fetchDefaultCollation(plan) match {
case Some(collation) =>
- transform(plan, StringType(collation))
- case None => plan
+ transform(preprocessedPlan, StringType(collation))
+ case None => preprocessedPlan
}
}
@@ -104,15 +110,15 @@ object ApplyDefaultCollationToStringType extends Rule[LogicalPlan] {
case p if isCreateOrAlterPlan(p) || AnalysisContext.get.collation.isDefined =>
transformPlan(p, newType)
- case addCols: AddColumns =>
+ case addCols@AddColumns(_: ResolvedTable, _) =>
addCols.copy(columnsToAdd = replaceColumnTypes(addCols.columnsToAdd, newType))
- case replaceCols: ReplaceColumns =>
+ case replaceCols@ReplaceColumns(_: ResolvedTable, _) =>
replaceCols.copy(columnsToAdd = replaceColumnTypes(replaceCols.columnsToAdd, newType))
- case a @ AlterColumns(_, specs: Seq[AlterColumnSpec]) =>
+ case a @ AlterColumns(ResolvedTable(_, _, table: Table, _), specs: Seq[AlterColumnSpec]) =>
val newSpecs = specs.map {
- case spec if spec.newDataType.isDefined && hasDefaultStringType(spec.newDataType.get) =>
+ case spec if shouldApplyDefaultCollationToAlterColumn(spec, table) =>
spec.copy(newDataType = Some(replaceDefaultStringType(spec.newDataType.get, newType)))
case col => col
}
@@ -120,6 +126,70 @@ object ApplyDefaultCollationToStringType extends Rule[LogicalPlan] {
}
}
+ /**
+ * The column type should not be changed if the original column type is [[StringType]] and the new
+ * type is the default [[StringType]] (i.e., [[StringType]] without an explicit collation).
+ *
+ * Query Example:
+ * {{{
+ * CREATE TABLE t (c1 STRING COLLATE UNICODE)
+ * ALTER TABLE t ALTER COLUMN c1 TYPE STRING -- c1 will remain STRING COLLATE UNICODE
+ * }}}
+ */
+ private def pruneRedundantAlterColumnTypes(plan: LogicalPlan): LogicalPlan = {
+ plan match {
+ case alterColumns@AlterColumns(
+ ResolvedTable(_, _, table: Table, _), specs: Seq[AlterColumnSpec]) =>
+ val resolvedSpecs = specs.map { spec =>
+ if (spec.newDataType.isDefined && isStringTypeColumn(spec.column, table) &&
+ isDefaultStringType(spec.newDataType.get)) {
+ spec.copy(newDataType = None)
+ } else {
+ spec
+ }
+ }
+ val newAlterColumns = CurrentOrigin.withOrigin(alterColumns.origin) {
+ alterColumns.copy(specs = resolvedSpecs)
+ }
+ newAlterColumns.copyTagsFrom(alterColumns)
+ newAlterColumns
+ case _ =>
+ plan
+ }
+ }
+
+ private def shouldApplyDefaultCollationToAlterColumn(
+ alterColumnSpec: AlterColumnSpec, table: Table): Boolean = {
+ alterColumnSpec.newDataType.isDefined &&
+ // Applies the default collation only if the original column's type is not StringType.
+ !isStringTypeColumn(alterColumnSpec.column, table) &&
+ hasDefaultStringType(alterColumnSpec.newDataType.get)
+ }
+
+ /**
+ * Checks whether the column's [[DataType]] is [[StringType]] in the given table. Throws an error
+ * if the column is not found.
+ */
+ private def isStringTypeColumn(fieldName: FieldName, table: Table): Boolean = {
+ CatalogV2Util.v2ColumnsToStructType(table.columns())
+ .findNestedField(fieldName.name, includeCollections = true, resolver = conf.resolver)
+ .map {
+ case (_, StructField(_, _: CharType, _, _)) =>
+ false
+ case (_, StructField(_, _: VarcharType, _, _)) =>
+ false
+ case (_, StructField(_, _: StringType, _, metadata))
+ if !metadata.contains(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY) =>
+ true
+ case (_, _) =>
+ false
+ }
+ .getOrElse {
+ throw QueryCompilationErrors.unresolvedColumnError(
+ toSQLId(fieldName.name), table.columns().map(_.name))
+ }
+ }
+
/**
* Transforms the given plan, by transforming all expressions in its operators to use the given
* new type instead of the default string type.
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/collation/DefaultCollationTestSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/collation/DefaultCollationTestSuite.scala
index 3373ab9a6dba..77ed8b411e3d 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/collation/DefaultCollationTestSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/collation/DefaultCollationTestSuite.scala
@@ -70,7 +70,7 @@ abstract class DefaultCollationTestSuite extends QueryTest with SharedSparkSessi
assertTableColumnCollation(testTable, "c2", "UNICODE")
sql(s"ALTER TABLE $testTable ALTER COLUMN c2 TYPE STRING")
- assertTableColumnCollation(testTable, "c2", "UTF8_BINARY")
+ assertTableColumnCollation(testTable, "c2", "UNICODE")
}
}
@@ -117,9 +117,25 @@ abstract class DefaultCollationTestSuite extends QueryTest with SharedSparkSessi
sql(s"ALTER TABLE $testTable ALTER COLUMN c1 TYPE STRING COLLATE UNICODE_CI")
assertTableColumnCollation(testTable, "c1", "UNICODE_CI")
+ // When using ALTER TABLE ALTER COLUMN TYPE, the column should inherit the table's collation
+ // only if it wasn't a string column before. If the column was already a string, and we're
+ // just changing its type to string (without explicit collation) again, keep the original
+ // collation.
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c1 TYPE STRING")
+ assertTableColumnCollation(testTable, "c1", "UNICODE_CI")
+
// alter table add columns with explicit collation, check collation for each column
sql(s"ALTER TABLE $testTable ADD COLUMN c7 STRING COLLATE SR_CI_AI")
sql(s"ALTER TABLE $testTable ADD COLUMN c8 STRING COLLATE UTF8_BINARY")
+ assertTableColumnCollation(testTable, "c7", "SR_CI_AI")
+ assertTableColumnCollation(testTable, "c8", "UTF8_BINARY")
+
+ // When using ALTER TABLE ALTER COLUMN TYPE, the column should inherit the table's collation
+ // only if it wasn't a string column before. If the column was already a string, and we're
+ // just changing its type to string (without explicit collation) again, keep the original
+ // collation.
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c8 TYPE STRING")
+
assertTableColumnCollation(testTable, "c1", "UNICODE_CI")
assertTableColumnCollation(testTable, "c2", "SR")
assertTableColumnCollation(testTable, "c3", "UTF8_BINARY")
@@ -131,6 +147,24 @@ abstract class DefaultCollationTestSuite extends QueryTest with SharedSparkSessi
}
}
+ test("Alter table alter column type with default collation") {
+ // When using ALTER TABLE ALTER COLUMN TYPE, the column should inherit the table's collation
+ // only if it wasn't a string column before. If the column was already a string, and we're
+ // just changing its type to string (without explicit collation) again, keep the original
+ // collation.
+ withTable(testTable) {
+ sql(s"CREATE TABLE $testTable (c1 STRING, c2 STRING COLLATE UTF8_LCASE, c3 STRING)" +
+ s" DEFAULT COLLATION UTF8_LCASE")
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c1 TYPE STRING")
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c2 TYPE STRING")
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c3 TYPE STRING COLLATE UNICODE")
+
+ assertTableColumnCollation(testTable, "c1", "UTF8_LCASE")
+ assertTableColumnCollation(testTable, "c2", "UTF8_LCASE")
+ assertTableColumnCollation(testTable, "c3", "UNICODE")
+ }
+ }
+
test("create table as select") {
// literals in select do not pick up session collation
withTable(testTable) {
@@ -444,4 +478,26 @@ class DefaultCollationTestSuiteV2 extends DefaultCollationTestSuite with Datasou
checkAnswer(sql(s"SELECT DISTINCT COLLATION(c2) FROM $testTable"), Row(s"$prefix.UTF8_LCASE"))
}
}
+
+ test("alter char/varchar column to string type") {
+ withTable(testTable) {
+ sql(s"CREATE TABLE $testTable (c1 VARCHAR(10), c2 CHAR(10)) " +
+ s"DEFAULT COLLATION UTF8_LCASE")
+
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c1 TYPE STRING")
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c2 TYPE STRING")
+ assertTableColumnCollation(testTable, "c1", "UTF8_LCASE")
+ assertTableColumnCollation(testTable, "c2", "UTF8_LCASE")
+ }
+
+ withTable(testTable) {
+ sql(s"CREATE TABLE $testTable (c1 VARCHAR(10), c2 CHAR(10)) " +
+ s"DEFAULT COLLATION UTF8_LCASE")
+
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c1 TYPE STRING COLLATE UNICODE")
+ sql(s"ALTER TABLE $testTable ALTER COLUMN c2 TYPE STRING COLLATE UNICODE")
+ assertTableColumnCollation(testTable, "c1", "UNICODE")
+ assertTableColumnCollation(testTable, "c2", "UNICODE")
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org