This is an automated email from the ASF dual-hosted git repository.
cloud-fan 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 893622534aea [SPARK-54831][SQL] Support COMMENT ON COLUMN for table
columns
893622534aea is described below
commit 893622534aea57bb64d067f41949b99645797011
Author: Ganesha Shreedhara <[email protected]>
AuthorDate: Mon Jul 20 22:26:36 2026 +0800
[SPARK-54831][SQL] Support COMMENT ON COLUMN for table columns
Add SQL support for `COMMENT ON COLUMN`, in two forms:
- `COMMENT ON COLUMN table.column IS 'comment'` - the last identifier part
is the (top-level) column and the rest is the table path.
- `COMMENT ON TABLE table COLUMN (col1 IS 'c1', col2 IS 'c2', ...)` -
comment multiple columns in one command; supports nested fields (e.g.
`point.x`) at any depth.
Both forms parse into an `AlterColumns` plan over an `UnresolvedTable`, so
a view target is cleanly rejected with `EXPECT_TABLE_NOT_VIEW`, consistent with
`COMMENT ON TABLE` and `ALTER TABLE ... ALTER COLUMN`.
A new error condition
`INVALID_SQL_SYNTAX.COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER` is raised when the
single-column form is given a bare column name.
A SQL reference doc page is added for the `COMMENT ON` statement.
`COMMENT ON COLUMN` is a common DDL command for documenting columns; it was
previously unsupported in Spark SQL.
New parser tests in `DDLParserSuite` and end-to-end tests in
`DataSourceV2SQLSuite` covering V1/V2 catalogs, multiple columns, nested
fields, `IS NULL`/empty comments, view rejection, and the nested-field boundary
of the single-column form.
### What changes were proposed in this pull request?
This PR adds SQL support for `COMMENT ON COLUMN` to set, update, and remove
comments on table columns, in two forms:
- Single column:
```sql
COMMENT ON COLUMN [catalog.][database.]table.column IS { 'comment' | NULL
}
The last identifier part is the (top-level) column name; the preceding
parts
qualify the table.
- Multiple columns / nested fields, on one table:
COMMENT ON TABLE table COLUMN ( col1 IS 'c1', col2.field IS 'c2', ... )
- Each column may be a nested field path (e.g. point.x) at any depth.
IS NULL (or an empty string) clears the comment, consistent with
COMMENT ON TABLE.
Implementation notes:
- Grammar: two new #commentColumn alternatives in SqlBaseParser.g4, plus
columnCommentList / columnComment rules. No new lexer tokens are
introduced (COLUMN, COMMENT, TABLE, etc. already exist).
- AstBuilder.visitCommentColumn builds an AlterColumns plan over an
UnresolvedTable, reusing the existing ALTER TABLE ... ALTER COLUMN
resolution and execution path (ResolveFieldNameAndPosition, CheckAnalysis,
DataSourceV2Strategy). Because the target is an UnresolvedTable, a view is
rejected cleanly with EXPECT_TABLE_NOT_VIEW, matching
COMMENT ON TABLE and ALTER TABLE ... ALTER COLUMN.
- The single-column form requires at least two identifier parts; a bare
column
name raises the new error condition
INVALID_SQL_SYNTAX.COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER.
- Adds a SQL reference doc page for the COMMENT ON statement.
This supersedes the earlier #53593 (closed); that version added a dedicated
CommentOnColumn plan node, whereas this reuses the existing AlterColumns
infrastructure and additionally supports the multi-column / nested-field
form.
### Why are the changes needed?
Currently, Spark only supports setting column comments via:
1. CREATE TABLE with a COMMENT clause, or
2. ALTER TABLE ... ALTER COLUMN ... COMMENT (which cannot remove a comment).
COMMENT ON COLUMN provides standard SQL syntax that:
- follows SQL-standard conventions, consistent with the existing
COMMENT ON TABLE / COMMENT ON NAMESPACE commands,
- allows explicit removal of a column comment via IS NULL, and
- supports commenting multiple columns (and nested fields) in one statement.
### Does this PR introduce _any_ user-facing change?
Yes. It adds the COMMENT ON COLUMN and COMMENT ON TABLE ... COLUMN (...) SQL
commands and a corresponding SQL reference documentation page. There is no
change
to existing behavior.
### How was this patch tested?
New unit tests:
- DDLParserSuite: parsing of both syntax forms, identifier splitting,
IS NULL / empty / literal 'NULL' comments, nested-field paths, and the
INVALID_SQL_SYNTAX.COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER error.
- DataSourceV2SQLSuite: end-to-end coverage over V1 (session) and V2
(non-session) catalogs — setting/updating/removing comments, multiple
columns,
deeply nested fields, view rejection (EXPECT_TABLE_NOT_VIEW), unknown column
(UNRESOLVED_COLUMN), the parent/child same-command restriction
(NOT_SUPPORTED_CHANGE_SAME_COLUMN), and the nested-field boundary of the
single-column form.
All added tests pass locally via
build/sbt "catalyst/testOnly *DDLParserSuite" and
build/sbt "sql/testOnly *DataSourceV2SQLSuiteV1Filter".
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
Closes #57311 from ganeshashree/SPARK-54831.
Lead-authored-by: Ganesha Shreedhara <[email protected]>
Co-authored-by: Ganesha S <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
---
.../src/main/resources/error/error-conditions.json | 5 +
docs/sql-migration-guide.md | 1 +
docs/sql-ref-syntax-ddl-comment.md | 96 ++++++
docs/sql-ref-syntax.md | 1 +
.../spark/sql/catalyst/parser/SqlBaseParser.g4 | 11 +
.../org/apache/spark/sql/types/StructField.scala | 11 +
.../spark/sql/connector/catalog/TableChange.java | 15 +-
.../spark/sql/catalyst/analysis/Analyzer.scala | 4 +-
.../catalyst/analysis/ApplyDefaultCollation.scala | 4 +-
.../sql/catalyst/analysis/CheckAnalysis.scala | 27 +-
.../sql/catalyst/analysis/TypeCoercionBase.scala | 2 +-
.../spark/sql/catalyst/parser/AstBuilder.scala | 48 +++
.../plans/logical/v2AlterTableCommands.scala | 36 +-
.../sql/connector/catalog/CatalogV2Util.scala | 6 +-
.../spark/sql/catalyst/parser/DDLParserSuite.scala | 81 +++++
.../catalyst/analysis/ResolveSessionCatalog.scala | 42 ++-
.../apache/spark/sql/execution/command/ddl.scala | 53 ++-
.../analyzer-results/change-column.sql.out | 12 +-
.../sql-tests/analyzer-results/charvarchar.sql.out | 6 +-
.../identifier-clause-legacy.sql.out | 6 +
.../analyzer-results/identifier-clause.sql.out | 8 +-
.../sql-tests/inputs/identifier-clause.sql | 1 +
.../results/identifier-clause-legacy.sql.out | 8 +
.../sql-tests/results/identifier-clause.sql.out | 8 +
.../spark/sql/connector/DataSourceV2SQLSuite.scala | 382 ++++++++++++++++++++-
.../execution/command/PlanResolutionSuite.scala | 23 +-
26 files changed, 840 insertions(+), 57 deletions(-)
diff --git a/common/utils/src/main/resources/error/error-conditions.json
b/common/utils/src/main/resources/error/error-conditions.json
index 444a4ec0aee0..6b38eb1c23e3 100644
--- a/common/utils/src/main/resources/error/error-conditions.json
+++ b/common/utils/src/main/resources/error/error-conditions.json
@@ -4886,6 +4886,11 @@
"ANALYZE TABLE(S) ... COMPUTE STATISTICS ... <ctx> must be either
NOSCAN or empty."
]
},
+ "COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER" : {
+ "message" : [
+ "COMMENT ON COLUMN requires an identifier with at least two parts.
Please qualify the column with the table name (e.g. `table.column`,
`database.table.column`, or `catalog.database.table.column`)."
+ ]
+ },
"CREATE_FUNC_WITH_COLUMN_CONSTRAINTS" : {
"message" : [
"CREATE FUNCTION with constraints on parameters is not allowed."
diff --git a/docs/sql-migration-guide.md b/docs/sql-migration-guide.md
index fe19681d3357..2607f6d0e56b 100644
--- a/docs/sql-migration-guide.md
+++ b/docs/sql-migration-guide.md
@@ -29,6 +29,7 @@ license: |
- Since Spark 4.3, the Spark Thrift Server rejects setting JVM system
properties through the `set:system:` session configuration overlay (for
example, in a JDBC connection string). To restore the previous behavior, set
`spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties` to `true`.
- Since Spark 4.3, the adaptive execution rule
`org.apache.spark.sql.execution.adaptive.DynamicJoinSelection` has been renamed
to `DemoteBroadcastHashJoin`, which now only demotes broadcast hash joins
(emitting `NO_BROADCAST_HASH`). Its selection of shuffled hash join over sort
merge join has moved to a new physical rule gated by
`spark.sql.adaptive.convertSortMergeJoinToShuffledHashJoin.enabled` (default
`true`). If you previously disabled the shuffled-hash-join preference by
listing `o [...]
- Since Spark 4.3, the exact `percentile`, `percentile_cont`, and `median`
aggregate functions (including their `WITHIN GROUP (ORDER BY ...)` forms)
compute the linear interpolation between two neighboring values as `lower +
fraction * (higher - lower)` instead of `(1 - fraction) * lower + fraction *
higher`. The two are equal in exact arithmetic, but the new form is
monotonically non-decreasing in the requested percentage and avoids a rounding
error the old form could introduce. As a re [...]
+- Since Spark 4.3, the new `COMMENT ON COLUMN ... IS NULL` syntax removes a
column comment by passing a `null` comment to
`TableChange.updateColumnComment(String[], String)`, so a `UpdateColumnComment`
table change may now carry a `null` `newComment()`. Previously `newComment()`
was always non-null. DataSource V2 catalogs that handle `UpdateColumnComment`
should null-check `newComment()` and treat `null` as "remove the column
comment" (mirroring how `UpdateColumnDefaultValue` already car [...]
## Upgrading from Spark SQL 4.1 to 4.2
diff --git a/docs/sql-ref-syntax-ddl-comment.md
b/docs/sql-ref-syntax-ddl-comment.md
new file mode 100644
index 000000000000..397327cb4b7f
--- /dev/null
+++ b/docs/sql-ref-syntax-ddl-comment.md
@@ -0,0 +1,96 @@
+---
+layout: global
+title: COMMENT ON
+displayTitle: COMMENT ON
+license: |
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+---
+
+### Description
+
+The `COMMENT ON` statement sets or updates the comment on a database, a table,
or
+one or more table columns. Setting a column comment to `NULL` removes it.
+
+### Syntax
+
+```sql
+COMMENT ON { DATABASE | SCHEMA | NAMESPACE } database_name IS comment
+
+COMMENT ON TABLE table_name IS comment
+
+COMMENT ON COLUMN table_name.column_name IS comment
+
+COMMENT ON TABLE table_name COLUMN ( column_name IS comment [ , ... ] )
+```
+
+### Parameters
+
+* **database_name**
+
+ Specifies the name of the database on which the comment is set.
+
+* **table_name**
+
+ Specifies the name of the table on which the comment is set. The name may
be
+ qualified with a catalog and/or database name.
+
+* **column_name**
+
+ Specifies the column on which the comment is set. In the single-column form
+ (`COMMENT ON COLUMN`), the identifier must have at least two parts: the
last
+ part is the column name and the preceding parts qualify the table (for
example
+ `table_name.column_name` or `database_name.table_name.column_name`). This
form
+ can only reference a top-level column. To comment on a nested field, or on
+ multiple columns at once, use the `COMMENT ON TABLE table_name COLUMN (
... )`
+ form, where each `column_name` may be a nested field path such as
`col.field`.
+
+ A parent field and one of its nested fields cannot be commented in the same
+ `COMMENT ON TABLE ... COLUMN` statement.
+
+ The target of a column comment must be a table; views are not supported.
+
+* **comment**
+
+ A `STRING` literal or `NULL`. For a column, setting the comment to `NULL`
removes
+ an existing comment, while an empty string `''` sets an empty comment. For
a database
+ or table, both `NULL` and `''` set an empty comment.
+
+### Examples
+
+```sql
+-- Set a comment on a database
+COMMENT ON DATABASE some_db IS 'This is a database';
+
+-- Set a comment on a table
+COMMENT ON TABLE some_db.some_table IS 'This is a table';
+
+-- Set a comment on a single column
+COMMENT ON COLUMN some_db.some_table.some_column IS 'This is a column';
+
+-- Remove a column comment
+COMMENT ON COLUMN some_db.some_table.some_column IS NULL;
+
+-- Set comments on multiple columns, including a nested field
+COMMENT ON TABLE some_db.some_table COLUMN (
+ id IS 'The identifier',
+ address.city IS 'The city part of the address'
+);
+```
+
+### Related Statements
+
+* [ALTER TABLE](sql-ref-syntax-ddl-alter-table.html)
+* [DESCRIBE TABLE](sql-ref-syntax-aux-describe-table.html)
diff --git a/docs/sql-ref-syntax.md b/docs/sql-ref-syntax.md
index 222c507423f8..cf123f677d26 100644
--- a/docs/sql-ref-syntax.md
+++ b/docs/sql-ref-syntax.md
@@ -28,6 +28,7 @@ Data Definition Statements are used to create or modify the
structure of databas
* [ALTER DATABASE](sql-ref-syntax-ddl-alter-database.html)
* [ALTER TABLE](sql-ref-syntax-ddl-alter-table.html)
* [ALTER VIEW](sql-ref-syntax-ddl-alter-view.html)
+ * [COMMENT ON](sql-ref-syntax-ddl-comment.html)
* [CREATE DATABASE](sql-ref-syntax-ddl-create-database.html)
* [CREATE FUNCTION (External)](sql-ref-syntax-ddl-create-function.html)
* [CREATE FUNCTION (SQL)](sql-ref-syntax-ddl-create-sql-function.html)
diff --git
a/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
b/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
index fd0bba2f9684..e64d88cce1ec 100644
---
a/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
+++
b/sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
@@ -395,6 +395,9 @@ statement
| COMMENT ON namespace identifierReference IS
comment
#commentNamespace
| COMMENT ON TABLE identifierReference IS comment
#commentTable
+ | COMMENT ON TABLE identifierReference COLUMN
+ LEFT_PAREN columns=columnCommentList RIGHT_PAREN
#commentColumn
+ | COMMENT ON COLUMN columnComment
#commentColumn
| REFRESH TABLE identifierReference
#refreshTable
| REFRESH FUNCTION identifierReference
#refreshFunction
| REFRESH (stringLit | .*?)
#refreshResource
@@ -1938,6 +1941,14 @@ alterColumnAction
| dropDefault=DROP DEFAULT
;
+columnCommentList
+ : columnComment (COMMA columnComment)*
+ ;
+
+columnComment
+ : column=multipartIdentifier IS comment
+ ;
+
// Matches exactly one string literal without coalescing or parameter markers.
// Used in type constructors where coalescing is not allowed.
singleStringLitWithoutMarker
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/types/StructField.scala
b/sql/api/src/main/scala/org/apache/spark/sql/types/StructField.scala
index 5bb12cf80e45..255830f1ece6 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/types/StructField.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/types/StructField.scala
@@ -160,6 +160,17 @@ case class StructField(
copy(metadata = newMetadata)
}
+ /**
+ * Clears the StructField of its comment, if any.
+ */
+ def clearComment(): StructField = {
+ val newMetadata = new MetadataBuilder()
+ .withMetadata(metadata)
+ .remove("comment")
+ .build()
+ copy(metadata = newMetadata)
+ }
+
/**
* Return the comment of this StructField.
*/
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 2cf2414f8052..0072512136e0 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
@@ -200,10 +200,10 @@ public interface TableChange {
* If the field does not exist, the change will result in an {@link
IllegalArgumentException}.
*
* @param fieldNames field names of the column to update
- * @param newComment the new comment
+ * @param newComment the new comment, or null to remove the existing comment
* @return a TableChange for the update
*/
- static TableChange updateColumnComment(String[] fieldNames, String
newComment) {
+ static TableChange updateColumnComment(String[] fieldNames, @Nullable String
newComment) {
return new UpdateColumnComment(fieldNames, newComment);
}
@@ -693,6 +693,11 @@ public interface TableChange {
return fieldNames;
}
+ /**
+ * Returns the new comment of the column, or null to remove the existing
comment (making the
+ * comment genuinely absent, as opposed to setting it to an empty string).
+ */
+ @Nullable
public String newComment() {
return newComment;
}
@@ -703,7 +708,7 @@ public interface TableChange {
if (o == null || getClass() != o.getClass()) return false;
UpdateColumnComment that = (UpdateColumnComment) o;
return Arrays.equals(fieldNames, that.fieldNames) &&
- newComment.equals(that.newComment);
+ Objects.equals(newComment, that.newComment);
}
@Override
@@ -715,7 +720,9 @@ public interface TableChange {
@Override
public String toString() {
- return "ALTER COLUMN " + TableChange.fieldPath(fieldNames) + " COMMENT";
+ return newComment == null
+ ? "ALTER COLUMN " + TableChange.fieldPath(fieldNames) + " DROP
COMMENT"
+ : "ALTER COLUMN " + TableChange.fieldPath(fieldNames) + "
COMMENT";
}
}
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
index 14574916d1ac..51882a9dd983 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
@@ -4191,10 +4191,10 @@ class Analyzer(
resolved.copyTagsFrom(a)
resolved
- case a @ AlterColumns(table: ResolvedTable, specs) =>
+ case a @ AlterColumns(table: ResolvedTable, specs, _) =>
val resolvedSpecs = specs.map {
case s @ AlterColumnSpec(
- ResolvedFieldName(path, field), dataType, _, _, position, _, _)
=>
+ ResolvedFieldName(path, field), dataType, _, _, position, _, _,
_) =>
val newDataType = dataType.flatMap { dt =>
// Hive style syntax provides the column type, even if it may
not have changed.
val existing =
CharVarcharUtils.getRawType(field.metadata).getOrElse(field.dataType)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollation.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollation.scala
index 4485c299d218..ff43b3668839 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollation.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ApplyDefaultCollation.scala
@@ -267,7 +267,7 @@ object ApplyDefaultCollation extends Rule[LogicalPlan] {
case replaceCols: ReplaceColumns =>
replaceCols.copy(columnsToAdd =
replaceColumnTypes(replaceCols.columnsToAdd, collation))
- case a @ AlterColumns(ResolvedTable(_, _, _, _), specs:
Seq[AlterColumnSpec]) =>
+ case a @ AlterColumns(ResolvedTable(_, _, _, _), specs:
Seq[AlterColumnSpec], _) =>
val newSpecs = specs.map {
case spec if shouldApplyDefaultCollationToAlterColumn(spec) =>
spec.copy(newDataType =
@@ -292,7 +292,7 @@ object ApplyDefaultCollation extends Rule[LogicalPlan] {
private def pruneRedundantAlterColumnTypes(plan: LogicalPlan): LogicalPlan =
{
plan match {
case alterColumns@AlterColumns(
- ResolvedTable(_, _, _, _), specs: Seq[AlterColumnSpec]) =>
+ ResolvedTable(_, _, _, _), specs: Seq[AlterColumnSpec], _) =>
val resolvedSpecs = specs.map { spec =>
if (isAlterColumnNOP(spec)) {
spec.copy(newDataType = None)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
index ff482e940d3f..86e789b0b2b1 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
@@ -1255,27 +1255,16 @@ trait CheckAnalysis extends LookupCatalog with
QueryErrorsBase with PlanToString
case RenameColumn(table: ResolvedTable, col: ResolvedFieldName, newName)
=>
checkColumnNotExists("rename", col.path :+ newName, table.schema)
- case AlterColumns(table: ResolvedTable, specs) =>
- val groupedColumns = specs.groupBy(_.column.name)
- groupedColumns.collect {
- case (name, occurrences) if occurrences.length > 1 =>
- alter.failAnalysis(
- errorClass = "NOT_SUPPORTED_CHANGE_SAME_COLUMN",
- messageParameters = Map(
- "table" -> toSQLId(table.name),
- "fieldName" -> toSQLId(name)))
- }
- groupedColumns.keys.foreach { name =>
- if (groupedColumns.keys.exists(child => child != name &&
child.startsWith(name))) {
- alter.failAnalysis(
- errorClass = "NOT_SUPPORTED_CHANGE_SAME_COLUMN",
- messageParameters = Map(
- "table" -> toSQLId(table.name),
- "fieldName" -> toSQLId(name)))
- }
+ case AlterColumns(table: ResolvedTable, specs, _) =>
+ AlterColumns.findRepeatedColumn(specs).foreach { name =>
+ alter.failAnalysis(
+ errorClass = "NOT_SUPPORTED_CHANGE_SAME_COLUMN",
+ messageParameters = Map(
+ "table" -> toSQLId(table.name),
+ "fieldName" -> toSQLId(name)))
}
specs.foreach {
- case AlterColumnSpec(col: ResolvedFieldName, dataType, nullable, _,
_, _, _) =>
+ case AlterColumnSpec(col: ResolvedFieldName, dataType, nullable, _,
_, _, _, _) =>
val fieldName = col.name.quoted
if (dataType.isDefined) {
val field = CharVarcharUtils.getRawType(col.field.metadata)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionBase.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionBase.scala
index f8175e971ed0..7c83a5282910 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionBase.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionBase.scala
@@ -134,7 +134,7 @@ abstract class TypeCoercionBase extends TypeCoercionHelper {
}
addColumns.copy(columnsToAdd = newCols)
- case alterColumns @ AlterColumns(_, specs) if alterColumns.resolved &&
+ case alterColumns @ AlterColumns(_, specs, _) if alterColumns.resolved &&
specs.exists(_.newDefaultExpression.isDefined) =>
val newSpecs = specs.map { c =>
val dataType =
c.column.asInstanceOf[ResolvedFieldName].field.dataType
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
index 4f525e810b8f..fe3e7dd25d33 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
@@ -7288,6 +7288,54 @@ class AstBuilder extends DataTypeAstBuilder
CommentOnTable(createUnresolvedTable(ctx.identifierReference, "COMMENT ON
TABLE"), comment)
}
+ override def visitCommentColumn(ctx: CommentColumnContext): LogicalPlan =
withOrigin(ctx) {
+ // Each column comment is either a new comment value, or `IS NULL` which
removes the existing
+ // comment. `IS NULL` and `IS ''` are distinguished here: the former
yields None (removal), the
+ // latter Some("") (an empty comment). The `comment` grammar rule is
`stringLit | NULL`, so a
+ // missing `stringLit()` means `IS NULL`.
+ def commentOf(c: CommentContext): Option[String] =
+ Option(c.stringLit()).map(s => string(visitStringLit(s)))
+
+ def alterColumnSpec(column: Seq[String], comment: Option[String]):
AlterColumnSpec =
+ AlterColumnSpec(
+ UnresolvedFieldName(column),
+ newDataType = None,
+ newNullability = None,
+ newComment = comment,
+ newPosition = None,
+ newDefaultExpression = None,
+ dropComment = comment.isEmpty)
+
+ // Only tables are supported: downstream rules
(ResolveFieldNameAndPosition, CheckAnalysis,
+ // DataSourceV2Strategy) all expect a ResolvedTable, so a view target must
be rejected during
+ // resolution. UnresolvedTable resolves only to a table and reports
EXPECT_TABLE_NOT_VIEW
+ // otherwise, matching COMMENT ON TABLE and ALTER TABLE ... ALTER COLUMN.
+ if (ctx.columnComment != null) {
+ // COMMENT ON COLUMN table.column IS 'comment'
+ val identifiers = visitMultipartIdentifier(ctx.columnComment.column)
+ if (identifiers.size < 2) {
+ throw new
ParseException("INVALID_SQL_SYNTAX.COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER", ctx)
+ }
+ val tableId = identifiers.dropRight(1)
+ val columnId = identifiers.takeRight(1)
+ val spec = alterColumnSpec(columnId,
commentOf(ctx.columnComment.comment))
+ AlterColumns(
+ UnresolvedTable(tableId, "COMMENT ON COLUMN"), Seq(spec),
fromCommentOn = true)
+ } else {
+ // COMMENT ON TABLE table COLUMN (column1 IS 'comment1', column2 IS
'comment2', ...)
+ // The table target is an identifierReference, so it also supports
IDENTIFIER(...) clauses.
+ val specs = ctx.columns.columnComment.asScala.map { c =>
+ alterColumnSpec(visitMultipartIdentifier(c.column),
commentOf(c.comment))
+ }.toSeq
+ withIdentClause(
+ ctx.identifierReference,
+ tableId =>
+ AlterColumns(
+ UnresolvedTable(tableId, "COMMENT ON TABLE ... COLUMN"), specs,
+ fromCommentOn = true))
+ }
+ }
+
override def visitComment (ctx: CommentContext): String = {
Option(ctx.stringLit()).map(s => string(visitStringLit(s))).getOrElse("")
}
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 68b3573ce5ce..66d1cd6f2df2 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
@@ -218,6 +218,8 @@ case class RenameColumn(
* @param newPosition new position of column if set
* @param newDefaultExpression new default expression if set
* @param dropDefault whether to drop the default expression
+ * @param dropComment whether to remove the existing comment (making it
genuinely absent, as
+ * opposed to setting it to an empty string)
*/
case class AlterColumnSpec(
column: FieldName,
@@ -226,7 +228,8 @@ case class AlterColumnSpec(
newComment: Option[String],
newPosition: Option[FieldPosition],
newDefaultExpression: Option[DefaultValueExpression],
- dropDefault: Boolean = false) extends Expression with Unevaluable {
+ dropDefault: Boolean = false,
+ dropComment: Boolean = false) extends Expression with Unevaluable {
override def children: Seq[Expression] = Seq(column) ++ newPosition.toSeq ++
newDefaultExpression.toSeq
@@ -259,10 +262,16 @@ case class AlterColumnSpec(
/**
* The logical plan of the ALTER TABLE ... ALTER COLUMN command.
+ *
+ * `fromCommentOn` records whether this plan was produced by a `COMMENT ON ...
COLUMN` statement
+ * (as opposed to `ALTER TABLE ... ALTER COLUMN`). Both syntaxes build the
same plan, but the V1
+ * session-catalog routing only extends the multi-column / nested-field
comment support to the
+ * `COMMENT ON` form, so it must be able to tell the two apart after
resolution.
*/
case class AlterColumns(
table: LogicalPlan,
- specs: Seq[AlterColumnSpec]) extends AlterTableCommand {
+ specs: Seq[AlterColumnSpec],
+ fromCommentOn: Boolean = false) extends AlterTableCommand {
override def changes: Seq[TableChange] = {
specs.flatMap { spec =>
val column = spec.column
@@ -274,8 +283,12 @@ case class AlterColumns(
val nullabilityChange = spec.newNullability.map { nullable =>
TableChange.updateColumnNullability(colName, nullable)
}
- val commentChange = spec.newComment.map { newComment =>
- TableChange.updateColumnComment(colName, newComment)
+ val commentChange = if (spec.dropComment) {
+ Some(TableChange.updateColumnComment(colName, null: String))
+ } else {
+ spec.newComment.map { newComment =>
+ TableChange.updateColumnComment(colName, newComment)
+ }
}
val positionChange = spec.newPosition.map { newPosition =>
require(newPosition.resolved,
@@ -301,6 +314,21 @@ case class AlterColumns(
copy(table = newChild)
}
+object AlterColumns {
+ /**
+ * Returns the name of the first column that is altered more than once,
either directly (the same
+ * column appears in multiple specs) or through a parent/descendant
relationship (both a field and
+ * one of its nested fields are altered). Returns None when every column is
altered at most once.
+ * Used to reject `ALTER COLUMN` / `COMMENT ON ... COLUMN` statements that
change the same column
+ * twice, which is not supported.
+ */
+ def findRepeatedColumn(specs: Seq[AlterColumnSpec]): Option[Seq[String]] = {
+ val names = specs.map(_.column.name)
+ names.find(name => names.count(_ == name) > 1)
+ .orElse(names.find(name => names.exists(other => other != name &&
other.startsWith(name))))
+ }
+}
+
/**
* The logical plan of the following commands:
* - ALTER TABLE ... CLUSTER BY (col1, col2, ...)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
index a4b136d33e6f..c0905ede4f0d 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
@@ -265,7 +265,11 @@ private[sql] object CatalogV2Util {
case update: UpdateColumnComment =>
replace(schema, update.fieldNames.toImmutableArraySeq, field =>
- Some(field.withComment(update.newComment)))
+ if (update.newComment != null) {
+ Some(field.withComment(update.newComment))
+ } else {
+ Some(field.clearComment())
+ })
case update: UpdateColumnPosition =>
def updateFieldPos(struct: StructType, name: String): StructType = {
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala
index f8d16cb0097c..4870c118dbc7 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala
@@ -3093,6 +3093,87 @@ class DDLParserSuite extends AnalysisTest {
comparePlans(
parsePlan("COMMENT ON TABLE a.b.c IS 'xYz'"),
CommentOnTable(UnresolvedTable(Seq("a", "b", "c"), "COMMENT ON TABLE"),
"xYz"))
+
+ // `comment` is the new comment value, or None for `IS NULL` (which
removes the comment).
+ def alterColumnComment(
+ tableIdent: Seq[String],
+ columnComments: Seq[(Seq[String], Option[String])],
+ commandName: String): AlterColumns = {
+ AlterColumns(
+ UnresolvedTable(tableIdent, commandName),
+ columnComments.map { case (column, comment) =>
+ AlterColumnSpec(
+ UnresolvedFieldName(column),
+ newDataType = None,
+ newNullability = None,
+ newComment = comment,
+ newPosition = None,
+ newDefaultExpression = None,
+ dropComment = comment.isEmpty)
+ },
+ fromCommentOn = true)
+ }
+
+ // COMMENT ON COLUMN: last identifier part is the column name
+ Seq(
+ ("a.b", Seq("a"), Seq("b")),
+ ("a.b.c", Seq("a", "b"), Seq("c")),
+ ("a.b.c.d", Seq("a", "b", "c"), Seq("d"))
+ ).foreach { case (columnIdentifier, tableIdent, columnIdent) =>
+ comparePlans(
+ parsePlan(s"COMMENT ON COLUMN $columnIdentifier IS 'comment'"),
+ alterColumnComment(tableIdent, Seq((columnIdent, Some("comment"))),
"COMMENT ON COLUMN"))
+ }
+
+ // COMMENT ON COLUMN: IS NULL removes the comment (dropComment), while IS
'' sets an empty one.
+ comparePlans(
+ parsePlan("COMMENT ON COLUMN a.b.c.d IS NULL"),
+ alterColumnComment(Seq("a", "b", "c"), Seq((Seq("d"), None)), "COMMENT
ON COLUMN"))
+ comparePlans(
+ parsePlan("COMMENT ON COLUMN a.b.c.d IS ''"),
+ alterColumnComment(Seq("a", "b", "c"), Seq((Seq("d"), Some(""))),
"COMMENT ON COLUMN"))
+ comparePlans(
+ parsePlan("COMMENT ON COLUMN a.b.c.d IS 'NULL'"),
+ alterColumnComment(Seq("a", "b", "c"), Seq((Seq("d"), Some("NULL"))),
"COMMENT ON COLUMN"))
+
+ // COMMENT ON COLUMN requires at least two identifier parts.
+ checkError(
+ exception = parseException("COMMENT ON COLUMN a IS 'comment'"),
+ condition = "INVALID_SQL_SYNTAX.COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER",
+ parameters = Map.empty,
+ context = ExpectedContext(
+ fragment = "COMMENT ON COLUMN a IS 'comment'",
+ start = 0,
+ stop = 31))
+
+ // COMMENT ON TABLE ... COLUMN (...) supports multiple columns and nested
fields.
+ comparePlans(
+ parsePlan("COMMENT ON TABLE a.b COLUMN (c IS 'comment')"),
+ alterColumnComment(
+ Seq("a", "b"),
+ Seq((Seq("c"), Some("comment"))),
+ "COMMENT ON TABLE ... COLUMN"))
+ comparePlans(
+ parsePlan(
+ """COMMENT ON TABLE a.b COLUMN (
+ | c IS 'comment 1',
+ | c.d IS 'comment 2',
+ | c.d.e IS 'comment 3')""".stripMargin),
+ alterColumnComment(
+ Seq("a", "b"),
+ Seq(
+ (Seq("c"), Some("comment 1")),
+ (Seq("c", "d"), Some("comment 2")),
+ (Seq("c", "d", "e"), Some("comment 3"))),
+ "COMMENT ON TABLE ... COLUMN"))
+
+ // COMMENT ON TABLE ... COLUMN: IS NULL removes a column comment.
+ comparePlans(
+ parsePlan("COMMENT ON TABLE a.b COLUMN (c IS NULL, d IS 'keep')"),
+ alterColumnComment(
+ Seq("a", "b"),
+ Seq((Seq("c"), None), (Seq("d"), Some("keep"))),
+ "COMMENT ON TABLE ... COLUMN"))
}
test("create table - without using") {
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
index 8d483bb0c95b..2ed873427e28 100644
---
a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
+++
b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
@@ -81,7 +81,30 @@ class ResolveSessionCatalog(val catalogManager:
CatalogManager)
case ReplaceColumns(ResolvedV1TableIdentifier(ident), _) =>
throw QueryCompilationErrors.unsupportedTableOperationError(ident,
"REPLACE COLUMNS")
- case a @ AlterColumns(ResolvedTable(catalog, ident, table: V1Table, _),
specs)
+ // Comment-only changes that touch multiple columns or a nested field (e.g.
+ // `COMMENT ON TABLE t COLUMN (a IS 'x', b IS 'y')` or `... (point.x IS
'x')`) cannot be
+ // expressed by the single-column, top-level-only
`AlterTableChangeColumnCommand`. Handle them
+ // with a dedicated V1 command that applies all specs at once via
`applySchemaChanges`.
+ // This is scoped to the `COMMENT ON ... COLUMN` syntax (`fromCommentOn`);
the equivalent
+ // `ALTER TABLE ... ALTER COLUMN` multi-column / nested-field forms still
fall through to the
+ // next case and keep their existing `UNSUPPORTED_FEATURE.TABLE_OPERATION`
error on V1.
+ case a @ AlterColumns(rt @ ResolvedTable(catalog, _, table: V1Table, _),
specs, _)
+ if a.fromCommentOn && supportsV1Command(catalog) && a.resolved &&
+ isMultiOrNestedCommentOnly(specs) =>
+ // This rule rewrites AlterColumns into a command during resolution,
before CheckAnalysis
+ // gets to validate it, so re-run the "same column changed twice" check
here (the same one
+ // CheckAnalysis applies to the AlterColumns node on the V2 path). Use
`a.failAnalysis` so the
+ // error carries the same query context as the V2 path.
+ AlterColumns.findRepeatedColumn(specs).foreach { name =>
+ a.failAnalysis(
+ errorClass = "NOT_SUPPORTED_CHANGE_SAME_COLUMN",
+ messageParameters = Map(
+ "table" -> toSQLId(rt.name),
+ "fieldName" -> toSQLId(name)))
+ }
+ AlterTableChangeColumnCommentsCommand(table.catalogTable.identifier,
a.changes)
+
+ case a @ AlterColumns(ResolvedTable(catalog, ident, table: V1Table, _),
specs, _)
if supportsV1Command(catalog) && a.resolved &&
specs.forall(_.isDefaultValueTypeCoerced) =>
if (specs.size > 1) {
throw QueryCompilationErrors.unsupportedTableOperationError(
@@ -135,7 +158,8 @@ class ResolveSessionCatalog(val catalogManager:
CatalogManager)
dataType,
nullable = true,
builder.build())
- AlterTableChangeColumnCommand(table.catalogTable.identifier, colName,
newColumn)
+ AlterTableChangeColumnCommand(
+ table.catalogTable.identifier, colName, newColumn, dropComment =
s.dropComment)
case AlterTableClusterBy(ResolvedTable(catalog, _, table: V1Table, _),
clusterBySpecOpt)
if supportsV1Command(catalog) =>
@@ -987,4 +1011,18 @@ class ResolveSessionCatalog(val catalogManager:
CatalogManager)
catalog.isInstanceOf[CatalogExtension])
}
+ // True when the ALTER COLUMN specs only change comments (set or drop, no
type / nullability /
+ // position / default changes) and, taken together, touch multiple columns
or a nested field.
+ // Such changes come from `COMMENT ON TABLE ... COLUMN (...)` and cannot be
expressed by the
+ // single-column, top-level-only V1 `AlterTableChangeColumnCommand`, so they
are handled by the
+ // dedicated `AlterTableChangeColumnCommentsCommand` instead.
+ private def isMultiOrNestedCommentOnly(specs: Seq[AlterColumnSpec]): Boolean
= {
+ val commentOnly = specs.forall { s =>
+ (s.newComment.isDefined || s.dropComment) &&
+ s.newDataType.isEmpty && s.newNullability.isEmpty &&
s.newPosition.isEmpty &&
+ s.newDefaultExpression.isEmpty && !s.dropDefault
+ }
+ commentOnly && (specs.size > 1 || specs.exists(_.column.name.length > 1))
+ }
+
}
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
index fe3ec61ce0f1..01f74180f38e 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
@@ -41,7 +41,7 @@ import
org.apache.spark.sql.catalyst.types.DataTypeUtils.toAttributes
import
org.apache.spark.sql.catalyst.util.ResolveDefaultColumnsUtils.CURRENT_DEFAULT_COLUMN_METADATA_KEY
import org.apache.spark.sql.catalyst.util.TypeUtils.toSQLId
import org.apache.spark.sql.classic.ClassicConversions.castToImpl
-import org.apache.spark.sql.connector.catalog.{CatalogManager, CatalogV2Util,
Identifier, TableCatalog}
+import org.apache.spark.sql.connector.catalog.{CatalogManager, CatalogV2Util,
Identifier, TableCatalog, TableChange}
import
org.apache.spark.sql.connector.catalog.CatalogManager.SESSION_CATALOG_NAME
import org.apache.spark.sql.connector.catalog.SupportsNamespaces._
import org.apache.spark.sql.errors.QueryCompilationErrors
@@ -381,7 +381,8 @@ case class AlterTableUnsetPropertiesCommand(
case class AlterTableChangeColumnCommand(
tableName: TableIdentifier,
columnName: String,
- newColumn: StructField) extends LeafRunnableCommand {
+ newColumn: StructField,
+ dropComment: Boolean = false) extends LeafRunnableCommand {
// TODO: support change column name/dataType/metadata/position.
override def run(sparkSession: SparkSession): Seq[Row] = {
@@ -420,9 +421,15 @@ case class AlterTableChangeColumnCommand(
val newDataSchema = table.dataSchema.fields.map { field =>
if (field.name == originColumn.name) {
- // Create a new column from the origin column with the new type and
new comment.
- val withNewTypeAndComment: StructField =
- addComment(withNewType(field, newColumn.dataType),
newColumn.getComment())
+ // Create a new column from the origin column with the new type and
new comment. When
+ // `dropComment` is set (COMMENT ON COLUMN ... IS NULL), the comment
is removed entirely
+ // rather than kept or set to an empty string.
+ val typedField = withNewType(field, newColumn.dataType)
+ val withNewTypeAndComment: StructField = if (dropComment) {
+ typedField.clearComment()
+ } else {
+ addComment(typedField, newColumn.getComment())
+ }
// Create a new column from the origin column with the new current
default value.
// The default value is already validated by ResolveSessionCatalog, so
we just need
// to copy the CURRENT_DEFAULT metadata. Note: we preserve the
original EXISTS_DEFAULT
@@ -489,6 +496,42 @@ case class AlterTableChangeColumnCommand(
}
}
+/**
+ * A command to set or remove comments on one or more columns of a table,
including nested fields.
+ * Used for the multi-column and nested-field forms of `COMMENT ON TABLE ...
COLUMN (...)` on the
+ * session catalog, which the single-column [[AlterTableChangeColumnCommand]]
cannot express. Only
+ * `UpdateColumnComment` changes are expected here; the comments are applied
to the data schema via
+ * [[CatalogV2Util.applySchemaChanges]].
+ */
+case class AlterTableChangeColumnCommentsCommand(
+ tableName: TableIdentifier,
+ changes: Seq[TableChange]) extends LeafRunnableCommand {
+
+ override def run(sparkSession: SparkSession): Seq[Row] = {
+ val catalog = sparkSession.sessionState.catalog
+ catalog.refreshTable(tableName)
+ val table = catalog.getTableRawMetadata(tableName)
+ val resolver = sparkSession.sessionState.conf.resolver
+ DDLUtils.verifyAlterTableType(catalog, table, isView = false)
+
+ // Comments on partition columns are not supported by this path.
+ changes.foreach {
+ case c: TableChange.UpdateColumnComment =>
+ val topLevel = c.fieldNames.head
+ if (table.partitionColumnNames.exists(resolver(topLevel, _))) {
+ throw
QueryCompilationErrors.cannotAlterPartitionColumn(table.qualifiedName, topLevel)
+ }
+ case _ =>
+ }
+
+ val newDataSchema = CatalogV2Util.applySchemaChanges(
+ table.dataSchema, changes, table.provider, "ALTER TABLE")
+ catalog.alterTableDataSchema(tableName, newDataSchema)
+
+ Seq.empty[Row]
+ }
+}
+
/**
* A command that sets the serde class and/or serde properties of a table/view.
*
diff --git
a/sql/core/src/test/resources/sql-tests/analyzer-results/change-column.sql.out
b/sql/core/src/test/resources/sql-tests/analyzer-results/change-column.sql.out
index cdb2ef60aa3e..5451f358faee 100644
---
a/sql/core/src/test/resources/sql-tests/analyzer-results/change-column.sql.out
+++
b/sql/core/src/test/resources/sql-tests/analyzer-results/change-column.sql.out
@@ -128,19 +128,19 @@ DescribeTableCommand
`spark_catalog`.`default`.`test_change`, false, [col_name#x
-- !query
ALTER TABLE test_change CHANGE a COMMENT 'this is column a'
-- !query analysis
-AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true), false
-- !query
ALTER TABLE test_change CHANGE b COMMENT '#*02?`'
-- !query analysis
-AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, b,
StructField(b,StringType,true)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, b,
StructField(b,StringType,true), false
-- !query
ALTER TABLE test_change CHANGE c COMMENT ''
-- !query analysis
-AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, c,
StructField(c,IntegerType,true)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, c,
StructField(c,IntegerType,true), false
-- !query
@@ -153,13 +153,13 @@ DescribeTableCommand
`spark_catalog`.`default`.`test_change`, false, [col_name#x
-- !query
ALTER TABLE test_change CHANGE a TYPE INT
-- !query analysis
-AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true), false
-- !query
ALTER TABLE test_change CHANGE a COMMENT 'this is column a'
-- !query analysis
-AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true), false
-- !query
@@ -200,7 +200,7 @@ DescribeTableCommand
`spark_catalog`.`default`.`test_change`, false, [col_name#x
-- !query
ALTER TABLE test_change CHANGE A COMMENT 'case insensitivity'
-- !query analysis
-AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`test_change`, a,
StructField(a,IntegerType,true), false
-- !query
diff --git
a/sql/core/src/test/resources/sql-tests/analyzer-results/charvarchar.sql.out
b/sql/core/src/test/resources/sql-tests/analyzer-results/charvarchar.sql.out
index 8915835fffe8..67d9a1b8a0a5 100644
--- a/sql/core/src/test/resources/sql-tests/analyzer-results/charvarchar.sql.out
+++ b/sql/core/src/test/resources/sql-tests/analyzer-results/charvarchar.sql.out
@@ -152,7 +152,7 @@ org.apache.spark.sql.AnalysisException
-- !query
alter table char_tbl1 change column c type char(5)
-- !query analysis
-AlterTableChangeColumnCommand `spark_catalog`.`default`.`char_tbl1`, c,
StructField(c,CharType(5),true)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`char_tbl1`, c,
StructField(c,CharType(5),true), false
-- !query
@@ -280,13 +280,13 @@ DescribeTableCommand
`spark_catalog`.`default`.`char_part`, true, [col_name#x, d
-- !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)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`char_part`, c1,
StructField(c1,CharType(5),true), false
-- !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)
+AlterTableChangeColumnCommand `spark_catalog`.`default`.`char_part`, v1,
StructField(v1,VarcharType(6),true), false
-- !query
diff --git
a/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause-legacy.sql.out
b/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause-legacy.sql.out
index 8a69bd059764..4cecec03578a 100644
---
a/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause-legacy.sql.out
+++
b/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause-legacy.sql.out
@@ -1771,6 +1771,12 @@ org.apache.spark.sql.catalyst.parser.ParseException
}
+-- !query
+COMMENT ON TABLE IDENTIFIER('test_comment') COLUMN (c1 IS 'c1 comment', c2 IS
'c2 comment')
+-- !query analysis
+AlterTableChangeColumnCommentsCommand
`spark_catalog`.`identifier_clause_test_schema`.`test_comment`, [ALTER COLUMN
c1 COMMENT, ALTER COLUMN c2 COMMENT]
+
+
-- !query
DROP TABLE test_comment
-- !query analysis
diff --git
a/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause.sql.out
b/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause.sql.out
index d0caaa5b57ae..35ceffe4ac9f 100644
---
a/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause.sql.out
+++
b/sql/core/src/test/resources/sql-tests/analyzer-results/identifier-clause.sql.out
@@ -1674,7 +1674,13 @@ CommentOnTable table comment
-- !query
ALTER TABLE test_comment ALTER COLUMN IDENTIFIER('c1') COMMENT 'column comment'
-- !query analysis
-AlterTableChangeColumnCommand
`spark_catalog`.`identifier_clause_test_schema`.`test_comment`, c1,
StructField(c1,IntegerType,true)
+AlterTableChangeColumnCommand
`spark_catalog`.`identifier_clause_test_schema`.`test_comment`, c1,
StructField(c1,IntegerType,true), false
+
+
+-- !query
+COMMENT ON TABLE IDENTIFIER('test_comment') COLUMN (c1 IS 'c1 comment', c2 IS
'c2 comment')
+-- !query analysis
+AlterTableChangeColumnCommentsCommand
`spark_catalog`.`identifier_clause_test_schema`.`test_comment`, [ALTER COLUMN
c1 COMMENT, ALTER COLUMN c2 COMMENT]
-- !query
diff --git a/sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql
b/sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql
index d9bafe7cc607..1425004e9fcc 100644
--- a/sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql
+++ b/sql/core/src/test/resources/sql-tests/inputs/identifier-clause.sql
@@ -226,6 +226,7 @@ DROP TABLE test_desc;
CREATE TABLE test_comment(c1 INT, c2 STRING) USING CSV;
COMMENT ON TABLE IDENTIFIER('test_comment') IS 'table comment';
ALTER TABLE test_comment ALTER COLUMN IDENTIFIER('c1') COMMENT 'column
comment';
+COMMENT ON TABLE IDENTIFIER('test_comment') COLUMN (c1 IS 'c1 comment', c2 IS
'c2 comment');
DROP TABLE test_comment;
-- Additional identifier tests with qualified table names in various commands
diff --git
a/sql/core/src/test/resources/sql-tests/results/identifier-clause-legacy.sql.out
b/sql/core/src/test/resources/sql-tests/results/identifier-clause-legacy.sql.out
index 53a5971301cd..cc8bd806365b 100644
---
a/sql/core/src/test/resources/sql-tests/results/identifier-clause-legacy.sql.out
+++
b/sql/core/src/test/resources/sql-tests/results/identifier-clause-legacy.sql.out
@@ -1999,6 +1999,14 @@ org.apache.spark.sql.catalyst.parser.ParseException
}
+-- !query
+COMMENT ON TABLE IDENTIFIER('test_comment') COLUMN (c1 IS 'c1 comment', c2 IS
'c2 comment')
+-- !query schema
+struct<>
+-- !query output
+
+
+
-- !query
DROP TABLE test_comment
-- !query schema
diff --git
a/sql/core/src/test/resources/sql-tests/results/identifier-clause.sql.out
b/sql/core/src/test/resources/sql-tests/results/identifier-clause.sql.out
index 6537797d928a..a45c2da46ada 100644
--- a/sql/core/src/test/resources/sql-tests/results/identifier-clause.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/identifier-clause.sql.out
@@ -1861,6 +1861,14 @@ struct<>
+-- !query
+COMMENT ON TABLE IDENTIFIER('test_comment') COLUMN (c1 IS 'c1 comment', c2 IS
'c2 comment')
+-- !query schema
+struct<>
+-- !query output
+
+
+
-- !query
DROP TABLE test_comment
-- !query schema
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
index 4f855e7e453d..0896df430a42 100644
---
a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
+++
b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
@@ -50,7 +50,7 @@ import
org.apache.spark.sql.execution.streaming.runtime.MemoryStream
import org.apache.spark.sql.internal.{SQLConf, StaticSQLConf}
import org.apache.spark.sql.internal.SQLConf.{PARTITION_OVERWRITE_MODE,
PartitionOverwriteMode, V2_SESSION_CATALOG_IMPLEMENTATION}
import org.apache.spark.sql.sources.SimpleScanSource
-import org.apache.spark.sql.types.{IntegerType, LongType, StringType,
StructField, StructType}
+import org.apache.spark.sql.types.{ArrayType, CharType, IntegerType, LongType,
StringType, StructField, StructType, VarcharType}
import org.apache.spark.unsafe.types.UTF8String
abstract class DataSourceV2SQLSuite
@@ -3170,6 +3170,386 @@ class DataSourceV2SQLSuiteV1Filter
}
}
+ test("SPARK-54831: COMMENT ON COLUMN") {
+ spark.conf.set(V2_SESSION_CATALOG_IMPLEMENTATION, "builtin")
+
+ // Session catalog (V1) table.
+ withTable("t") {
+ sql("CREATE TABLE t(k int COMMENT 'original comment', v string) USING
json")
+
+ assert(columnComment("t", "k") === Some("original comment"))
+
+ // Update an existing comment.
+ sql("COMMENT ON COLUMN t.k IS 'new comment'")
+ assert(columnComment("t", "k") === Some("new comment"))
+
+ // IS NULL removes the comment entirely, so it becomes genuinely absent
(not an empty string).
+ sql("COMMENT ON COLUMN t.k IS NULL")
+ assert(columnComment("t", "k").isEmpty)
+
+ // Setting back to literal 'NULL' must keep the string value.
+ sql("COMMENT ON COLUMN t.k IS 'NULL'")
+ assert(columnComment("t", "k") === Some("NULL"))
+
+ // Empty string sets an empty comment, which is distinct from removing
it.
+ sql("COMMENT ON COLUMN t.k IS ''")
+ assert(columnComment("t", "k") === Some(""))
+
+ // Add comment to a column that didn't have one.
+ sql("COMMENT ON COLUMN t.v IS 'comment for v'")
+ assert(columnComment("t", "v") === Some("comment for v"))
+ }
+
+ // V2 non-session catalog table.
+ withTable("testcat.ns1.ns2.t") {
+ sql("CREATE TABLE testcat.ns1.ns2.t(k int COMMENT 'original', v string)
USING foo")
+ assert(columnComment("testcat.ns1.ns2.t", "k") === Some("original"))
+
+ sql("COMMENT ON COLUMN testcat.ns1.ns2.t.k IS 'updated comment'")
+ assert(columnComment("testcat.ns1.ns2.t", "k") === Some("updated
comment"))
+
+ // IS NULL removes the comment entirely.
+ sql("COMMENT ON COLUMN testcat.ns1.ns2.t.k IS NULL")
+ assert(columnComment("testcat.ns1.ns2.t", "k").isEmpty)
+
+ // IS '' sets an empty comment, which remains present.
+ sql("COMMENT ON COLUMN testcat.ns1.ns2.t.k IS ''")
+ assert(columnComment("testcat.ns1.ns2.t", "k") === Some(""))
+ }
+
+ // COMMENT ON COLUMN with only one identifier part is rejected.
+ val sql1 = "COMMENT ON COLUMN onlyone IS 'x'"
+ checkError(
+ exception = intercept[ParseException](sql(sql1)),
+ condition = "INVALID_SQL_SYNTAX.COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER",
+ parameters = Map.empty,
+ context = ExpectedContext(fragment = sql1, start = 0, stop = sql1.length
- 1))
+
+ // Unknown column is reported as an UNRESOLVED_COLUMN error.
+ withTable("t2") {
+ sql("CREATE TABLE t2(k int) USING json")
+ val sql2 = "COMMENT ON COLUMN t2.nonexistent IS 'test'"
+ checkError(
+ exception = analysisException(sql2),
+ condition = "UNRESOLVED_COLUMN.WITH_SUGGESTION",
+ parameters = Map("objectName" -> "`nonexistent`", "proposal" -> "`k`"),
+ context = ExpectedContext(fragment = sql2, start = 0, stop =
sql2.length - 1))
+ }
+
+ // Different qualification levels for the session catalog: table.column and
+ // database.table.column (matching the examples in the
+ // COMMENT_ON_COLUMN_INCORRECT_IDENTIFIER error message).
+ withDatabase("comment_test_db") {
+ sql("CREATE DATABASE comment_test_db")
+ sql("USE comment_test_db")
+ withTable("comment_test_db.qualified_test") {
+ sql("CREATE TABLE qualified_test(x int, y string, z int) USING json")
+
+ // table.column
+ sql("COMMENT ON COLUMN qualified_test.x IS 'with table.column'")
+ assert(columnComment("qualified_test", "x") === Some("with
table.column"))
+
+ // database.table.column
+ sql("COMMENT ON COLUMN comment_test_db.qualified_test.y IS 'with
db.table.column'")
+ assert(columnComment("qualified_test", "y") === Some("with
db.table.column"))
+
+ // catalog.database.table.column
+ sql(
+ "COMMENT ON COLUMN spark_catalog.comment_test_db.qualified_test.z IS
" +
+ "'with catalog.db.table.column'")
+ assert(columnComment("qualified_test", "z") === Some("with
catalog.db.table.column"))
+ }
+ }
+
+ // catalog.database.table.column against a V2 non-session catalog.
+ withTable("testcat.ns1.ns2.qualified") {
+ sql("CREATE TABLE testcat.ns1.ns2.qualified(c int) USING foo")
+ sql("COMMENT ON COLUMN testcat.ns1.ns2.qualified.c IS 'v2
catalog.db.table.column'")
+ assert(columnComment("testcat.ns1.ns2.qualified", "c") === Some("v2
catalog.db.table.column"))
+ }
+
+ // A view target is rejected: COMMENT ON COLUMN only supports tables.
+ withView("v") {
+ sql("CREATE VIEW v AS SELECT 1 AS c")
+ val sql3 = "COMMENT ON COLUMN v.c IS 'test'"
+ checkError(
+ exception = analysisException(sql3),
+ condition = "EXPECT_TABLE_NOT_VIEW.NO_ALTERNATIVE",
+ parameters = Map(
+ "viewName" -> "`spark_catalog`.`default`.`v`",
+ "operation" -> "COMMENT ON COLUMN"),
+ context = ExpectedContext(fragment = sql3, start = 0, stop =
sql3.length - 1))
+ }
+
+ // The single-column `COMMENT ON COLUMN` form cannot address a nested
field: the last
+ // identifier part is always taken as the (top-level) column name and
everything before it as
+ // the table path. So `t.point.x` is parsed as table
`testcat.ns1.ns2.t.point`, column `x`,
+ // and fails to find that table. Nested fields must use the `COMMENT ON
TABLE ... COLUMN (...)`
+ // form instead (covered below).
+ withTable("testcat.ns1.ns2.nested") {
+ sql("CREATE TABLE testcat.ns1.ns2.nested(point struct<x: double, y:
double>) USING foo")
+ val sql4 = "COMMENT ON COLUMN testcat.ns1.ns2.nested.point.x IS 'x
field'"
+ checkError(
+ exception = analysisException(sql4),
+ condition = "TABLE_OR_VIEW_NOT_FOUND",
+ parameters = Map("relationName" ->
"`testcat`.`ns1`.`ns2`.`nested`.`point`"),
+ context = ExpectedContext(fragment = sql4, start = 0, stop =
sql4.length - 1))
+ }
+ }
+
+ test("SPARK-54831: COMMENT ON TABLE ... COLUMN supports multiple columns and
nested fields") {
+ spark.conf.set(V2_SESSION_CATALOG_IMPLEMENTATION, "builtin")
+
+ withTable("testcat.ns1.ns2.t") {
+ sql(
+ "CREATE TABLE testcat.ns1.ns2.t(id int, data string, " +
+ "point struct<x: double, y: double>) USING foo")
+
+ sql(
+ """COMMENT ON TABLE testcat.ns1.ns2.t COLUMN (
+ | id IS 'id col',
+ | data IS 'data col',
+ | point IS 'point col')""".stripMargin)
+
+ assert(columnComment("testcat.ns1.ns2.t", "id") === Some("id col"))
+ assert(columnComment("testcat.ns1.ns2.t", "data") === Some("data col"))
+ assert(columnComment("testcat.ns1.ns2.t", "point") === Some("point col"))
+
+ // Nested fields must be commented in a separate statement because the
analyzer rejects
+ // commenting on a parent column and its nested fields in the same
command
+ // (NOT_SUPPORTED_CHANGE_SAME_COLUMN).
+ sql(
+ """COMMENT ON TABLE testcat.ns1.ns2.t COLUMN (
+ | point.x IS 'x field',
+ | point.y IS 'y field')""".stripMargin)
+
+ val pointField = spark.table("testcat.ns1.ns2.t").schema("point")
+ val struct = pointField.dataType.asInstanceOf[StructType]
+ assert(struct("x").getComment().contains("x field"))
+ assert(struct("y").getComment().contains("y field"))
+
+ // IS NULL removes a column comment, making it genuinely absent.
+ sql("COMMENT ON TABLE testcat.ns1.ns2.t COLUMN (id IS NULL)")
+ assert(columnComment("testcat.ns1.ns2.t", "id").isEmpty)
+
+ // A nested field comment can likewise be removed with IS NULL.
+ sql("COMMENT ON TABLE testcat.ns1.ns2.t COLUMN (point.x IS NULL)")
+ val pointAfter = spark.table("testcat.ns1.ns2.t").schema("point")
+ .dataType.asInstanceOf[StructType]
+ assert(pointAfter("x").getComment().isEmpty)
+ assert(pointAfter("y").getComment().contains("y field"))
+ }
+
+ // Deeply nested fields (struct within struct) can be commented via the
COLUMN (...) form.
+ // A parent field and its descendant cannot be changed in the same command
+ // (NOT_SUPPORTED_CHANGE_SAME_COLUMN), so comment them in separate
statements.
+ withTable("testcat.ns1.ns2.deep") {
+ sql(
+ "CREATE TABLE testcat.ns1.ns2.deep(" +
+ "outer struct<inner: struct<leaf: int>>) USING foo")
+
+ sql("COMMENT ON TABLE testcat.ns1.ns2.deep COLUMN (outer.inner IS 'inner
struct')")
+ sql("COMMENT ON TABLE testcat.ns1.ns2.deep COLUMN (outer.inner.leaf IS
'leaf field')")
+
+ val outerField = spark.table("testcat.ns1.ns2.deep").schema("outer")
+ val outerStruct = outerField.dataType.asInstanceOf[StructType]
+ assert(outerStruct("inner").getComment().contains("inner struct"))
+ val innerStruct = outerStruct("inner").dataType.asInstanceOf[StructType]
+ assert(innerStruct("leaf").getComment().contains("leaf field"))
+ }
+
+ // A nested field inside an array element (`points.element.y`) can be
commented and removed,
+ // matching the reach of `ALTER TABLE ... ALTER COLUMN points.element.y
COMMENT`.
+ withTable("testcat.ns1.ns2.arr") {
+ sql(
+ "CREATE TABLE testcat.ns1.ns2.arr(" +
+ "id int, points array<struct<x: double, y: double>>) USING foo")
+
+ def yField(): StructField = {
+ val elem = spark.table("testcat.ns1.ns2.arr").schema("points")
+
.dataType.asInstanceOf[ArrayType].elementType.asInstanceOf[StructType]
+ elem("y")
+ }
+
+ sql("COMMENT ON TABLE testcat.ns1.ns2.arr COLUMN (points.element.y IS 'y
in array')")
+ assert(yField().getComment().contains("y in array"))
+
+ // IS NULL removes the array-element field comment.
+ sql("COMMENT ON TABLE testcat.ns1.ns2.arr COLUMN (points.element.y IS
NULL)")
+ assert(yField().getComment().isEmpty)
+ }
+ }
+
+ test("SPARK-54831: COMMENT ON TABLE ... COLUMN works on the session catalog
(V1 table)") {
+ spark.conf.set(V2_SESSION_CATALOG_IMPLEMENTATION, "builtin")
+
+ // Multiple top-level columns and nested fields work on an ordinary
managed table, routed
+ // through the shared V2 execution path rather than the single-column V1
command.
+ withTable("t") {
+ sql("CREATE TABLE t(id int, data string, point struct<x: double, y:
double>) USING json")
+
+ // Multiple top-level columns at once.
+ sql(
+ """COMMENT ON TABLE t COLUMN (
+ | id IS 'id col',
+ | data IS 'data col')""".stripMargin)
+ assert(columnComment("t", "id") === Some("id col"))
+ assert(columnComment("t", "data") === Some("data col"))
+
+ // Nested fields.
+ sql(
+ """COMMENT ON TABLE t COLUMN (
+ | point.x IS 'x field',
+ | point.y IS 'y field')""".stripMargin)
+ val struct =
spark.table("t").schema("point").dataType.asInstanceOf[StructType]
+ assert(struct("x").getComment().contains("x field"))
+ assert(struct("y").getComment().contains("y field"))
+
+ // A single nested field via COMMENT ON TABLE ... COLUMN.
+ sql("COMMENT ON TABLE t COLUMN (point.x IS 'updated x')")
+ val struct2 =
spark.table("t").schema("point").dataType.asInstanceOf[StructType]
+ assert(struct2("x").getComment().contains("updated x"))
+
+ // IS NULL removes comments (top-level and nested) on the session
catalog too.
+ sql("COMMENT ON TABLE t COLUMN (id IS NULL, point.y IS NULL)")
+ assert(columnComment("t", "id").isEmpty)
+ val struct3 =
spark.table("t").schema("point").dataType.asInstanceOf[StructType]
+ assert(struct3("y").getComment().isEmpty)
+
+ // The new comment is visible immediately via DESCRIBE (the relation
cache is refreshed).
+ sql("COMMENT ON TABLE t COLUMN (data IS 'fresh comment')")
+ checkAnswer(
+ sql("DESCRIBE t data").filter("info_name =
'comment'").select("info_value"),
+ Row("fresh comment"))
+
+ // The table target accepts an IDENTIFIER(...) clause, like COMMENT ON
TABLE ... IS.
+ sql("COMMENT ON TABLE IDENTIFIER('t') COLUMN (id IS 'via identifier')")
+ assert(columnComment("t", "id") === Some("via identifier"))
+ }
+
+ // char/varchar columns keep their declared type while their comment is
updated.
+ withTable("t") {
+ sql("CREATE TABLE t(c char(5), v varchar(6)) USING parquet")
+ sql("COMMENT ON TABLE t COLUMN (c IS 'char comment', v IS 'varchar
comment')")
+ assert(columnComment("t", "c") === Some("char comment"))
+ assert(columnComment("t", "v") === Some("varchar comment"))
+ val raw = spark.sessionState.catalog.getTableRawMetadata(
+ TableIdentifier("t", Some("default")))
+ assert(raw.schema("c").dataType === CharType(5))
+ assert(raw.schema("v").dataType === VarcharType(6))
+ }
+
+ // A parent field and its nested field still cannot be changed in the same
command.
+ withTable("t") {
+ sql("CREATE TABLE t(point struct<x: double, y: double>) USING json")
+ val sqlText =
+ """COMMENT ON TABLE t COLUMN (
+ | point IS 'point col',
+ | point.x IS 'x field')""".stripMargin
+ checkError(
+ exception = intercept[AnalysisException](sql(sqlText)),
+ condition = "NOT_SUPPORTED_CHANGE_SAME_COLUMN",
+ parameters = Map(
+ "table" -> "`spark_catalog`.`default`.`t`",
+ "fieldName" -> "`point`"),
+ context = ExpectedContext(fragment = sqlText, start = 0, stop =
sqlText.length - 1))
+ }
+
+ // An unknown column is still reported as UNRESOLVED_COLUMN on the session
catalog.
+ withTable("t") {
+ sql("CREATE TABLE t(id int) USING json")
+ val sqlText = "COMMENT ON TABLE t COLUMN (id IS 'x', nope IS 'y')"
+ checkError(
+ exception = analysisException(sqlText),
+ condition = "UNRESOLVED_COLUMN.WITH_SUGGESTION",
+ parameters = Map("objectName" -> "`nope`", "proposal" -> "`id`"),
+ context = ExpectedContext(fragment = sqlText, start = 0, stop =
sqlText.length - 1))
+ }
+
+ // Commenting a partition column is rejected.
+ withTable("t") {
+ sql("CREATE TABLE t(id int, p string) USING json PARTITIONED BY (p)")
+ checkError(
+ exception = intercept[AnalysisException](
+ sql("COMMENT ON TABLE t COLUMN (id IS 'id col', p IS 'part col')")),
+ condition = "CANNOT_ALTER_PARTITION_COLUMN",
+ parameters = Map(
+ "tableName" -> "`spark_catalog`.`default`.`t`",
+ "columnName" -> "`p`"))
+ }
+ }
+
+ test("SPARK-54831: session-catalog V1 comment routing is scoped to COMMENT
ON ... COLUMN") {
+ spark.conf.set(V2_SESSION_CATALOG_IMPLEMENTATION, "builtin")
+
+ // The dedicated V1 comment command only handles the `COMMENT ON ...
COLUMN` syntax. The
+ // equivalent bulk / nested-field `ALTER TABLE ... ALTER COLUMN ...
COMMENT` forms build the
+ // same `AlterColumns` plan but keep their pre-existing
UNSUPPORTED_FEATURE.TABLE_OPERATION
+ // error on the session catalog, so this PR does not silently change ALTER
TABLE behavior.
+ withTable("t") {
+ sql("CREATE TABLE t(id int, data string, point struct<x: double, y:
double>) USING json")
+
+ // Bulk ALTER COLUMN comment change (multiple columns) is still
unsupported on V1.
+ checkError(
+ exception = intercept[AnalysisException](
+ sql("ALTER TABLE t ALTER COLUMN id COMMENT 'id col', data COMMENT
'data col'")),
+ condition = "UNSUPPORTED_FEATURE.TABLE_OPERATION",
+ sqlState = "0A000",
+ parameters = Map(
+ "tableName" -> "`spark_catalog`.`default`.`t`",
+ "operation" -> "ALTER COLUMN in bulk"))
+ assert(columnComment("t", "id").isEmpty)
+ assert(columnComment("t", "data").isEmpty)
+
+ // Nested-field ALTER COLUMN comment change is still unsupported on V1.
+ checkError(
+ exception = intercept[AnalysisException](
+ sql("ALTER TABLE t ALTER COLUMN point.x COMMENT 'x field'")),
+ condition = "UNSUPPORTED_FEATURE.TABLE_OPERATION",
+ sqlState = "0A000",
+ parameters = Map(
+ "tableName" -> "`spark_catalog`.`default`.`t`",
+ "operation" -> "ALTER COLUMN with qualified column"))
+ val struct =
spark.table("t").schema("point").dataType.asInstanceOf[StructType]
+ assert(struct("x").getComment().isEmpty)
+
+ // The single-column, top-level ALTER COLUMN comment change keeps
working on V1.
+ sql("ALTER TABLE t ALTER COLUMN id COMMENT 'id col'")
+ assert(columnComment("t", "id") === Some("id col"))
+ }
+ }
+
+ test("SPARK-54831: COMMENT ON TABLE ... COLUMN cannot change parent and
child fields in the " +
+ "same command") {
+ spark.conf.set(V2_SESSION_CATALOG_IMPLEMENTATION, "builtin")
+
+ withTable("testcat.ns1.ns2.t") {
+ sql(
+ "CREATE TABLE testcat.ns1.ns2.t(id int, " +
+ "point struct<x: double, y: double>) USING foo")
+
+ val sqlText =
+ """COMMENT ON TABLE testcat.ns1.ns2.t COLUMN (
+ | point IS 'point col',
+ | point.x IS 'x field')""".stripMargin
+ checkError(
+ exception = intercept[AnalysisException](sql(sqlText)),
+ condition = "NOT_SUPPORTED_CHANGE_SAME_COLUMN",
+ parameters = Map(
+ "table" -> "`testcat`.`ns1`.`ns2`.`t`",
+ "fieldName" -> "`point`"),
+ context = ExpectedContext(fragment = sqlText, start = 0, stop =
sqlText.length - 1))
+ }
+ }
+
+ // Returns the comment of a top-level column as stored in the table schema.
Reads it from the
+ // StructField metadata (not DESCRIBE, which renders both an absent and an
empty comment as the
+ // same empty string) so callers can distinguish a removed comment (None)
from an empty one
+ // (Some("")).
+ private def columnComment(tableName: String, columnName: String):
Option[String] = {
+ spark.table(tableName).schema(columnName).getComment()
+ }
+
test("SPARK-31015: star expression should work for qualified column names
for v2 tables") {
val t = "testcat.ns1.ns2.tbl"
withTable(t) {
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
index 6968a75f5eee..cdba60d9960d 100644
---
a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
+++
b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
@@ -1513,7 +1513,9 @@ class PlanResolutionSuite extends SharedSparkSession with
AnalysisTest {
None,
None,
None,
- false))) =>
+ false,
+ false)),
+ _) =>
assert(column.name == Seq("i"))
case _ => fail("expect AlterColumns")
}
@@ -1528,7 +1530,9 @@ class PlanResolutionSuite extends SharedSparkSession with
AnalysisTest {
Some("new comment"),
None,
None,
- false))) =>
+ false,
+ false)),
+ _) =>
assert(column.name == Seq("i"))
case _ => fail("expect AlterColumns")
}
@@ -1545,6 +1549,7 @@ class PlanResolutionSuite extends SharedSparkSession with
AnalysisTest {
None,
None,
None,
+ false,
false),
AlterColumnSpec(
column2: ResolvedFieldName,
@@ -1553,7 +1558,9 @@ class PlanResolutionSuite extends SharedSparkSession with
AnalysisTest {
None,
None,
Some(DefaultValueExpression(_, _, _)),
- false))) =>
+ false,
+ false)),
+ _) =>
assert(column1.name == Seq("i"))
assert(column2.name == Seq("s"))
case _ => fail("expect AlterColumns")
@@ -1641,7 +1648,9 @@ class PlanResolutionSuite extends SharedSparkSession with
AnalysisTest {
Some(comment),
None,
None,
- false))) =>
+ false,
+ false)),
+ _) =>
assert(comment == "an index")
case _ => fail("expect AlterTableAlterColumn with comment change only")
}
@@ -1656,7 +1665,9 @@ class PlanResolutionSuite extends SharedSparkSession with
AnalysisTest {
Some(comment),
None,
None,
- false))) =>
+ false,
+ false)),
+ _) =>
assert(comment == "an index")
assert(dataType == LongType)
case _ => fail("expect AlterTableAlterColumn with type and comment
changes")
@@ -1691,7 +1702,7 @@ class PlanResolutionSuite extends SharedSparkSession with
AnalysisTest {
val catalog = if (isSessionCatalog) v2SessionCatalog else testCat
val tableIdent = if (isSessionCatalog) "v2Table" else "tab"
parsed match {
- case AlterColumns(r: ResolvedTable, _) =>
+ case AlterColumns(r: ResolvedTable, _, _) =>
assert(r.catalog == catalog)
assert(r.identifier.name() == tableIdent)
case Project(_, AsDataSourceV2Relation(r)) =>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
