Skip to content

Commit

Permalink
[SPARK-49843][SQL] Fix change comment on char/varchar columns
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?

Fix the issue in `AlterTableChangeColumnCommand` where changing the comment of a char/varchar column also tries to change the column type to string.

### Why are the changes needed?

Because the newColumn will always be a `StringType` even when the metadata says that it was originally char/varchar.

### Does this PR introduce _any_ user-facing change?

Yes, the query will no longer fail when using this code path.

### How was this patch tested?

New query in golden files.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #48315 from stefankandic/fixAlterVarcharCol.

Authored-by: Stefan Kandic <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
  • Loading branch information
stefankandic authored and MaxGekk committed Oct 2, 2024
1 parent 3551a9e commit 077a319
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable
import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute}
import org.apache.spark.sql.catalyst.plans.logical._
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.catalyst.util.{quoteIfNeeded, toPrettySQL, ResolveDefaultColumns => DefaultCols}
import org.apache.spark.sql.catalyst.util.{quoteIfNeeded, toPrettySQL, CharVarcharUtils, ResolveDefaultColumns => DefaultCols}
import org.apache.spark.sql.catalyst.util.ResolveDefaultColumns._
import org.apache.spark.sql.connector.catalog.{CatalogExtension, CatalogManager, CatalogPlugin, CatalogV2Util, LookupCatalog, SupportsNamespaces, V1Table}
import org.apache.spark.sql.connector.expressions.Transform
Expand All @@ -36,7 +36,7 @@ import org.apache.spark.sql.execution.datasources.{CreateTable => CreateTableV1}
import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Utils
import org.apache.spark.sql.internal.{HiveSerDe, SQLConf}
import org.apache.spark.sql.internal.connector.V1Function
import org.apache.spark.sql.types.{MetadataBuilder, StructField, StructType}
import org.apache.spark.sql.types.{MetadataBuilder, StringType, StructField, StructType}
import org.apache.spark.util.ArrayImplicits._

/**
Expand Down Expand Up @@ -87,7 +87,11 @@ class ResolveSessionCatalog(val catalogManager: CatalogManager)
val colName = a.column.name(0)
val dataType = a.dataType.getOrElse {
table.schema.findNestedField(Seq(colName), resolver = conf.resolver)
.map(_._2.dataType)
.map {
case (_, StructField(_, st: StringType, _, metadata)) =>
CharVarcharUtils.getRawType(metadata).getOrElse(st)
case (_, field) => field.dataType
}
.getOrElse {
throw QueryCompilationErrors.unresolvedColumnError(
toSQLId(a.column.name), table.schema.fieldNames)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,18 @@ desc formatted char_part
DescribeTableCommand `spark_catalog`.`default`.`char_part`, true, [col_name#x, data_type#x, comment#x]


-- !query
alter table char_part change column c1 comment 'char comment'
-- !query analysis
AlterTableChangeColumnCommand `spark_catalog`.`default`.`char_part`, c1, StructField(c1,CharType(5),true)


-- !query
alter table char_part change column v1 comment 'varchar comment'
-- !query analysis
AlterTableChangeColumnCommand `spark_catalog`.`default`.`char_part`, v1, StructField(v1,VarcharType(6),true)


-- !query
alter table char_part add partition (v2='ke', c2='nt') location 'loc1'
-- !query analysis
Expand Down
2 changes: 2 additions & 0 deletions sql/core/src/test/resources/sql-tests/inputs/charvarchar.sql
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ desc formatted char_tbl1;
create table char_part(c1 char(5), c2 char(2), v1 varchar(6), v2 varchar(2)) using parquet partitioned by (v2, c2);
desc formatted char_part;

alter table char_part change column c1 comment 'char comment';
alter table char_part change column v1 comment 'varchar comment';
alter table char_part add partition (v2='ke', c2='nt') location 'loc1';
desc formatted char_part;

Expand Down
32 changes: 24 additions & 8 deletions sql/core/src/test/resources/sql-tests/results/charvarchar.sql.out
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,22 @@ Location [not included in comparison]/{warehouse_dir}/char_part
Partition Provider Catalog


-- !query
alter table char_part change column c1 comment 'char comment'
-- !query schema
struct<>
-- !query output



-- !query
alter table char_part change column v1 comment 'varchar comment'
-- !query schema
struct<>
-- !query output



-- !query
alter table char_part add partition (v2='ke', c2='nt') location 'loc1'
-- !query schema
Expand All @@ -569,8 +585,8 @@ desc formatted char_part
-- !query schema
struct<col_name:string,data_type:string,comment:string>
-- !query output
c1 char(5)
v1 varchar(6)
c1 char(5) char comment
v1 varchar(6) varchar comment
v2 varchar(2)
c2 char(2)
# Partition Information
Expand Down Expand Up @@ -612,8 +628,8 @@ desc formatted char_part
-- !query schema
struct<col_name:string,data_type:string,comment:string>
-- !query output
c1 char(5)
v1 varchar(6)
c1 char(5) char comment
v1 varchar(6) varchar comment
v2 varchar(2)
c2 char(2)
# Partition Information
Expand Down Expand Up @@ -647,8 +663,8 @@ desc formatted char_part
-- !query schema
struct<col_name:string,data_type:string,comment:string>
-- !query output
c1 char(5)
v1 varchar(6)
c1 char(5) char comment
v1 varchar(6) varchar comment
v2 varchar(2)
c2 char(2)
# Partition Information
Expand Down Expand Up @@ -682,8 +698,8 @@ desc formatted char_part
-- !query schema
struct<col_name:string,data_type:string,comment:string>
-- !query output
c1 char(5)
v1 varchar(6)
c1 char(5) char comment
v1 varchar(6) varchar comment
v2 varchar(2)
c2 char(2)
# Partition Information
Expand Down

0 comments on commit 077a319

Please sign in to comment.