This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 1185db494dda [SPARK-54377][SQL] Fix COMMENT ON TABLE IS NULL to
properly remove table comment
1185db494dda is described below
commit 1185db494dda3bdff350049d40a938c19b02c7b8
Author: Ganesha S <[email protected]>
AuthorDate: Tue Nov 18 16:42:05 2025 +0800
[SPARK-54377][SQL] Fix COMMENT ON TABLE IS NULL to properly remove table
comment
### What changes were proposed in this pull request?
This PR fixes a bug where COMMENT ON TABLE table_name IS NULL was not
properly removing the table comment.
### Why are the changes needed?
The syntax COMMENT ON TABLE table_name IS NULL should remove the table
comment. However, the previous implementation was setting the comment to null
rather than removing the property entirely.
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
Enhanced test case test("COMMENT ON TABLE") in DataSourceV2SQLSuite
verifies:
* Comment can be set and is stored correctly
* Comment is completely removed when set to NULL (property no longer exists)
* Literal string "NULL" can still be set as a comment value
* Works for both session catalog and V2 catalogs
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Sonnet 4.5
Closes #53091 from ganeshashree/SPARK-54377.
Authored-by: Ganesha S <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit e8f0a67e248d67e5e1951015a378c36282d12e17)
Signed-off-by: Wenchen Fan <[email protected]>
---
.../plans/logical/v2AlterTableCommands.scala | 6 +-
.../spark/sql/connector/DataSourceV2SQLSuite.scala | 71 ++++++++++++++++++++--
2 files changed, 72 insertions(+), 5 deletions(-)
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 d1eb561f3add..4ec8baf351cb 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
@@ -48,7 +48,11 @@ trait AlterTableCommand extends UnaryCommand {
*/
case class CommentOnTable(table: LogicalPlan, comment: String) extends
AlterTableCommand {
override def changes: Seq[TableChange] = {
- Seq(TableChange.setProperty(TableCatalog.PROP_COMMENT, comment))
+ if (comment == null) {
+ Seq(TableChange.removeProperty(TableCatalog.PROP_COMMENT))
+ } else {
+ Seq(TableChange.setProperty(TableCatalog.PROP_COMMENT, comment))
+ }
}
override protected def withNewChildInternal(newChild: LogicalPlan):
LogicalPlan =
copy(table = newChild)
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 907820e46238..847af570d6f3 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
@@ -2737,8 +2737,35 @@ class DataSourceV2SQLSuiteV1Filter
// Session catalog is used.
withTable("t") {
sql("CREATE TABLE t(k int) USING json")
+
+ // Verify no comment initially
+ val noCommentRows1 = sql("DESC EXTENDED t").toDF("k", "v", "c")
+ .where("k='Comment'")
+ .collect()
+ assert(noCommentRows1.isEmpty ||
noCommentRows1.head.getString(1).isEmpty,
+ "Expected no comment initially")
+
+ // Set a comment
checkTableComment("t", "minor revision")
+
+ // Verify comment is set
+ val commentRows1 = sql("DESC EXTENDED t").toDF("k", "v", "c")
+ .where("k='Comment'")
+ .collect()
+ assert(commentRows1.nonEmpty && commentRows1.head.getString(1) ===
"minor revision",
+ "Expected comment to be set to 'minor revision'")
+
+ // Remove comment by setting to NULL
checkTableComment("t", null)
+
+ // Verify comment is removed
+ val removedCommentRows1 = sql("DESC EXTENDED t").toDF("k", "v", "c")
+ .where("k='Comment'")
+ .collect()
+ assert(removedCommentRows1.isEmpty ||
removedCommentRows1.head.getString(1).isEmpty,
+ "Expected comment to be removed")
+
+ // Set comment to literal "NULL" string
checkTableComment("t", "NULL")
}
val sql1 = "COMMENT ON TABLE abc IS NULL"
@@ -2751,8 +2778,35 @@ class DataSourceV2SQLSuiteV1Filter
// V2 non-session catalog is used.
withTable("testcat.ns1.ns2.t") {
sql("CREATE TABLE testcat.ns1.ns2.t(k int) USING foo")
+
+ // Verify no comment initially
+ val noCommentRows2 = sql("DESC EXTENDED testcat.ns1.ns2.t").toDF("k",
"v", "c")
+ .where("k='Comment'")
+ .collect()
+ assert(noCommentRows2.isEmpty ||
noCommentRows2.head.getString(1).isEmpty,
+ "Expected no comment initially for testcat table")
+
+ // Set a comment
checkTableComment("testcat.ns1.ns2.t", "minor revision")
+
+ // Verify comment is set
+ val commentRows2 = sql("DESC EXTENDED testcat.ns1.ns2.t").toDF("k", "v",
"c")
+ .where("k='Comment'")
+ .collect()
+ assert(commentRows2.nonEmpty && commentRows2.head.getString(1) ===
"minor revision",
+ "Expected comment to be set to 'minor revision' for testcat table")
+
+ // Remove comment by setting to NULL
checkTableComment("testcat.ns1.ns2.t", null)
+
+ // Verify comment is removed
+ val removedCommentRows2 = sql("DESC EXTENDED
testcat.ns1.ns2.t").toDF("k", "v", "c")
+ .where("k='Comment'")
+ .collect()
+ assert(removedCommentRows2.isEmpty ||
removedCommentRows2.head.getString(1).isEmpty,
+ "Expected comment to be removed from testcat table")
+
+ // Set comment to literal "NULL" string
checkTableComment("testcat.ns1.ns2.t", "NULL")
}
val sql2 = "COMMENT ON TABLE testcat.abc IS NULL"
@@ -2778,10 +2832,19 @@ class DataSourceV2SQLSuiteV1Filter
private def checkTableComment(tableName: String, comment: String): Unit = {
sql(s"COMMENT ON TABLE $tableName IS " + Option(comment).map("'" + _ +
"'").getOrElse("NULL"))
- val expectedComment = Option(comment).getOrElse("")
- assert(sql(s"DESC extended $tableName").toDF("k", "v", "c")
- .where(s"k='${TableCatalog.PROP_COMMENT.capitalize}'")
- .head().getString(1) === expectedComment)
+ if (comment == null) {
+ // When comment is NULL, the property should be removed completely
+ val commentRows = sql(s"DESC extended $tableName").toDF("k", "v", "c")
+ .where("k='Comment'")
+ .collect()
+ assert(commentRows.isEmpty || commentRows.head.getString(1).isEmpty,
+ "Expected comment to be removed")
+ } else {
+ val expectedComment = comment
+ assert(sql(s"DESC extended $tableName").toDF("k", "v", "c")
+ .where("k='Comment'")
+ .head().getString(1) === expectedComment)
+ }
}
test("SPARK-31015: star expression should work for qualified column names
for v2 tables") {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
