Skip to content

Commit 1185db4

Browse files
ganeshas-dbcloud-fan
authored andcommitted
[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 <ganesha.s@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit e8f0a67) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
1 parent 6e78983 commit 1185db4

File tree

2 files changed

+72
-5
lines changed

2 files changed

+72
-5
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ trait AlterTableCommand extends UnaryCommand {
4848
*/
4949
case class CommentOnTable(table: LogicalPlan, comment: String) extends AlterTableCommand {
5050
override def changes: Seq[TableChange] = {
51-
Seq(TableChange.setProperty(TableCatalog.PROP_COMMENT, comment))
51+
if (comment == null) {
52+
Seq(TableChange.removeProperty(TableCatalog.PROP_COMMENT))
53+
} else {
54+
Seq(TableChange.setProperty(TableCatalog.PROP_COMMENT, comment))
55+
}
5256
}
5357
override protected def withNewChildInternal(newChild: LogicalPlan): LogicalPlan =
5458
copy(table = newChild)

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

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2737,8 +2737,35 @@ class DataSourceV2SQLSuiteV1Filter
27372737
// Session catalog is used.
27382738
withTable("t") {
27392739
sql("CREATE TABLE t(k int) USING json")
2740+
2741+
// Verify no comment initially
2742+
val noCommentRows1 = sql("DESC EXTENDED t").toDF("k", "v", "c")
2743+
.where("k='Comment'")
2744+
.collect()
2745+
assert(noCommentRows1.isEmpty || noCommentRows1.head.getString(1).isEmpty,
2746+
"Expected no comment initially")
2747+
2748+
// Set a comment
27402749
checkTableComment("t", "minor revision")
2750+
2751+
// Verify comment is set
2752+
val commentRows1 = sql("DESC EXTENDED t").toDF("k", "v", "c")
2753+
.where("k='Comment'")
2754+
.collect()
2755+
assert(commentRows1.nonEmpty && commentRows1.head.getString(1) === "minor revision",
2756+
"Expected comment to be set to 'minor revision'")
2757+
2758+
// Remove comment by setting to NULL
27412759
checkTableComment("t", null)
2760+
2761+
// Verify comment is removed
2762+
val removedCommentRows1 = sql("DESC EXTENDED t").toDF("k", "v", "c")
2763+
.where("k='Comment'")
2764+
.collect()
2765+
assert(removedCommentRows1.isEmpty || removedCommentRows1.head.getString(1).isEmpty,
2766+
"Expected comment to be removed")
2767+
2768+
// Set comment to literal "NULL" string
27422769
checkTableComment("t", "NULL")
27432770
}
27442771
val sql1 = "COMMENT ON TABLE abc IS NULL"
@@ -2751,8 +2778,35 @@ class DataSourceV2SQLSuiteV1Filter
27512778
// V2 non-session catalog is used.
27522779
withTable("testcat.ns1.ns2.t") {
27532780
sql("CREATE TABLE testcat.ns1.ns2.t(k int) USING foo")
2781+
2782+
// Verify no comment initially
2783+
val noCommentRows2 = sql("DESC EXTENDED testcat.ns1.ns2.t").toDF("k", "v", "c")
2784+
.where("k='Comment'")
2785+
.collect()
2786+
assert(noCommentRows2.isEmpty || noCommentRows2.head.getString(1).isEmpty,
2787+
"Expected no comment initially for testcat table")
2788+
2789+
// Set a comment
27542790
checkTableComment("testcat.ns1.ns2.t", "minor revision")
2791+
2792+
// Verify comment is set
2793+
val commentRows2 = sql("DESC EXTENDED testcat.ns1.ns2.t").toDF("k", "v", "c")
2794+
.where("k='Comment'")
2795+
.collect()
2796+
assert(commentRows2.nonEmpty && commentRows2.head.getString(1) === "minor revision",
2797+
"Expected comment to be set to 'minor revision' for testcat table")
2798+
2799+
// Remove comment by setting to NULL
27552800
checkTableComment("testcat.ns1.ns2.t", null)
2801+
2802+
// Verify comment is removed
2803+
val removedCommentRows2 = sql("DESC EXTENDED testcat.ns1.ns2.t").toDF("k", "v", "c")
2804+
.where("k='Comment'")
2805+
.collect()
2806+
assert(removedCommentRows2.isEmpty || removedCommentRows2.head.getString(1).isEmpty,
2807+
"Expected comment to be removed from testcat table")
2808+
2809+
// Set comment to literal "NULL" string
27562810
checkTableComment("testcat.ns1.ns2.t", "NULL")
27572811
}
27582812
val sql2 = "COMMENT ON TABLE testcat.abc IS NULL"
@@ -2778,10 +2832,19 @@ class DataSourceV2SQLSuiteV1Filter
27782832

27792833
private def checkTableComment(tableName: String, comment: String): Unit = {
27802834
sql(s"COMMENT ON TABLE $tableName IS " + Option(comment).map("'" + _ + "'").getOrElse("NULL"))
2781-
val expectedComment = Option(comment).getOrElse("")
2782-
assert(sql(s"DESC extended $tableName").toDF("k", "v", "c")
2783-
.where(s"k='${TableCatalog.PROP_COMMENT.capitalize}'")
2784-
.head().getString(1) === expectedComment)
2835+
if (comment == null) {
2836+
// When comment is NULL, the property should be removed completely
2837+
val commentRows = sql(s"DESC extended $tableName").toDF("k", "v", "c")
2838+
.where("k='Comment'")
2839+
.collect()
2840+
assert(commentRows.isEmpty || commentRows.head.getString(1).isEmpty,
2841+
"Expected comment to be removed")
2842+
} else {
2843+
val expectedComment = comment
2844+
assert(sql(s"DESC extended $tableName").toDF("k", "v", "c")
2845+
.where("k='Comment'")
2846+
.head().getString(1) === expectedComment)
2847+
}
27852848
}
27862849

27872850
test("SPARK-31015: star expression should work for qualified column names for v2 tables") {

0 commit comments

Comments
 (0)