From 42cc6d13edbebb7c435ec47c0c12b445e05fdd49 Mon Sep 17 00:00:00 2001
From: sujith71955 <sujithchacko.2010@gmail.com>
Date: Sun, 7 May 2017 23:15:00 -0700
Subject: [PATCH] [SPARK-20380][SQL] Unable to set/unset table comment property
using ALTER TABLE SET/UNSET TBLPROPERTIES ddl
### What changes were proposed in this pull request?
Table comment was not getting set/unset using **ALTER TABLE SET/UNSET TBLPROPERTIES** query
eg: ALTER TABLE table_with_comment SET TBLPROPERTIES("comment"= "modified comment)
when user alter the table properties and adds/updates table comment,table comment which is a field of **CatalogTable** instance is not getting updated and old table comment if exists was shown to user, inorder to handle this issue, update the comment field value in **CatalogTable** with the newly added/modified comment along with other table level properties when user executes **ALTER TABLE SET TBLPROPERTIES** query.
This pr has also taken care of unsetting the table comment when user executes query **ALTER TABLE UNSET TBLPROPERTIES** inorder to unset or remove table comment.
eg: ALTER TABLE table_comment UNSET TBLPROPERTIES IF EXISTS ('comment')
### How was this patch tested?
Added test cases as part of **SQLQueryTestSuite** for verifying table comment using desc formatted table query after adding/modifying table comment as part of **AlterTableSetPropertiesCommand** and unsetting the table comment using **AlterTableUnsetPropertiesCommand**.
Author: sujith71955 <sujithchacko.2010@gmail.com>
Closes #17649 from sujith71955/alter_table_comment.
---
.../catalyst/catalog/InMemoryCatalog.scala | 8 +-
.../spark/sql/execution/command/ddl.scala | 12 +-
.../describe-table-after-alter-table.sql | 29 ++++
.../describe-table-after-alter-table.sql.out | 161 ++++++++++++++++++
4 files changed, 204 insertions(+), 6 deletions(-)
create mode 100644 sql/core/src/test/resources/sql-tests/inputs/describe-table-after-alter-table.sql
create mode 100644 sql/core/src/test/resources/sql-tests/results/describe-table-after-alter-table.sql.out
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala
index 81dd8efc00..8a5319bebe 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/InMemoryCatalog.scala
@@ -216,8 +216,8 @@ class InMemoryCatalog(
} else {
tableDefinition
}
-
- catalog(db).tables.put(table, new TableDesc(tableWithLocation))
+ val tableProp = tableWithLocation.properties.filter(_._1 != "comment")
+ catalog(db).tables.put(table, new TableDesc(tableWithLocation.copy(properties = tableProp)))
}
}
@@ -298,7 +298,9 @@ class InMemoryCatalog(
assert(tableDefinition.identifier.database.isDefined)
val db = tableDefinition.identifier.database.get
requireTableExists(db, tableDefinition.identifier.table)
- catalog(db).tables(tableDefinition.identifier.table).table = tableDefinition
+ val updatedProperties = tableDefinition.properties.filter(kv => kv._1 != "comment")
+ val newTableDefinition = tableDefinition.copy(properties = updatedProperties)
+ catalog(db).tables(tableDefinition.identifier.table).table = newTableDefinition
}
override def alterTableSchema(
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 55540563ef..793fb9b795 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
@@ -231,8 +231,12 @@ case class AlterTableSetPropertiesCommand(
val catalog = sparkSession.sessionState.catalog
val table = catalog.getTableMetadata(tableName)
DDLUtils.verifyAlterTableType(catalog, table, isView)
- // This overrides old properties
- val newTable = table.copy(properties = table.properties ++ properties)
+ // This overrides old properties and update the comment parameter of CatalogTable
+ // with the newly added/modified comment since CatalogTable also holds comment as its
+ // direct property.
+ val newTable = table.copy(
+ properties = table.properties ++ properties,
+ comment = properties.get("comment"))
catalog.alterTable(newTable)
Seq.empty[Row]
}
@@ -267,8 +271,10 @@ case class AlterTableUnsetPropertiesCommand(
}
}
}
+ // If comment is in the table property, we reset it to None
+ val tableComment = if (propKeys.contains("comment")) None else table.properties.get("comment")
val newProperties = table.properties.filter { case (k, _) => !propKeys.contains(k) }
- val newTable = table.copy(properties = newProperties)
+ val newTable = table.copy(properties = newProperties, comment = tableComment)
catalog.alterTable(newTable)
Seq.empty[Row]
}
diff --git a/sql/core/src/test/resources/sql-tests/inputs/describe-table-after-alter-table.sql b/sql/core/src/test/resources/sql-tests/inputs/describe-table-after-alter-table.sql
new file mode 100644
index 0000000000..69bff6656c
--- /dev/null
+++ b/sql/core/src/test/resources/sql-tests/inputs/describe-table-after-alter-table.sql
@@ -0,0 +1,29 @@
+CREATE TABLE table_with_comment (a STRING, b INT, c STRING, d STRING) USING parquet COMMENT 'added';
+
+DESC FORMATTED table_with_comment;
+
+-- ALTER TABLE BY MODIFYING COMMENT
+ALTER TABLE table_with_comment SET TBLPROPERTIES("comment"= "modified comment", "type"= "parquet");
+
+DESC FORMATTED table_with_comment;
+
+-- DROP TEST TABLE
+DROP TABLE table_with_comment;
+
+-- CREATE TABLE WITHOUT COMMENT
+CREATE TABLE table_comment (a STRING, b INT) USING parquet;
+
+DESC FORMATTED table_comment;
+
+-- ALTER TABLE BY ADDING COMMENT
+ALTER TABLE table_comment SET TBLPROPERTIES(comment = "added comment");
+
+DESC formatted table_comment;
+
+-- ALTER UNSET PROPERTIES COMMENT
+ALTER TABLE table_comment UNSET TBLPROPERTIES IF EXISTS ('comment');
+
+DESC FORMATTED table_comment;
+
+-- DROP TEST TABLE
+DROP TABLE table_comment;
diff --git a/sql/core/src/test/resources/sql-tests/results/describe-table-after-alter-table.sql.out b/sql/core/src/test/resources/sql-tests/results/describe-table-after-alter-table.sql.out
new file mode 100644
index 0000000000..1cc11c475b
--- /dev/null
+++ b/sql/core/src/test/resources/sql-tests/results/describe-table-after-alter-table.sql.out
@@ -0,0 +1,161 @@
+-- Automatically generated by SQLQueryTestSuite
+-- Number of queries: 12
+
+
+-- !query 0
+CREATE TABLE table_with_comment (a STRING, b INT, c STRING, d STRING) USING parquet COMMENT 'added'
+-- !query 0 schema
+struct<>
+-- !query 0 output
+
+
+
+-- !query 1
+DESC FORMATTED table_with_comment
+-- !query 1 schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query 1 output
+# col_name data_type comment
+a string
+b int
+c string
+d string
+
+# Detailed Table Information
+Database default
+Table table_with_comment
+Created [not included in comparison]
+Last Access [not included in comparison]
+Type MANAGED
+Provider parquet
+Comment added
+Location [not included in comparison]sql/core/spark-warehouse/table_with_comment
+
+
+-- !query 2
+ALTER TABLE table_with_comment SET TBLPROPERTIES("comment"= "modified comment", "type"= "parquet")
+-- !query 2 schema
+struct<>
+-- !query 2 output
+
+
+
+-- !query 3
+DESC FORMATTED table_with_comment
+-- !query 3 schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query 3 output
+# col_name data_type comment
+a string
+b int
+c string
+d string
+
+# Detailed Table Information
+Database default
+Table table_with_comment
+Created [not included in comparison]
+Last Access [not included in comparison]
+Type MANAGED
+Provider parquet
+Comment modified comment
+Properties [type=parquet]
+Location [not included in comparison]sql/core/spark-warehouse/table_with_comment
+
+
+-- !query 4
+DROP TABLE table_with_comment
+-- !query 4 schema
+struct<>
+-- !query 4 output
+
+
+
+-- !query 5
+CREATE TABLE table_comment (a STRING, b INT) USING parquet
+-- !query 5 schema
+struct<>
+-- !query 5 output
+
+
+
+-- !query 6
+DESC FORMATTED table_comment
+-- !query 6 schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query 6 output
+# col_name data_type comment
+a string
+b int
+
+# Detailed Table Information
+Database default
+Table table_comment
+Created [not included in comparison]
+Last Access [not included in comparison]
+Type MANAGED
+Provider parquet
+Location [not included in comparison]sql/core/spark-warehouse/table_comment
+
+
+-- !query 7
+ALTER TABLE table_comment SET TBLPROPERTIES(comment = "added comment")
+-- !query 7 schema
+struct<>
+-- !query 7 output
+
+
+
+-- !query 8
+DESC formatted table_comment
+-- !query 8 schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query 8 output
+# col_name data_type comment
+a string
+b int
+
+# Detailed Table Information
+Database default
+Table table_comment
+Created [not included in comparison]
+Last Access [not included in comparison]
+Type MANAGED
+Provider parquet
+Comment added comment
+Location [not included in comparison]sql/core/spark-warehouse/table_comment
+
+
+-- !query 9
+ALTER TABLE table_comment UNSET TBLPROPERTIES IF EXISTS ('comment')
+-- !query 9 schema
+struct<>
+-- !query 9 output
+
+
+
+-- !query 10
+DESC FORMATTED table_comment
+-- !query 10 schema
+struct<col_name:string,data_type:string,comment:string>
+-- !query 10 output
+# col_name data_type comment
+a string
+b int
+
+# Detailed Table Information
+Database default
+Table table_comment
+Created [not included in comparison]
+Last Access [not included in comparison]
+Type MANAGED
+Provider parquet
+Location [not included in comparison]sql/core/spark-warehouse/table_comment
+
+
+-- !query 11
+DROP TABLE table_comment
+-- !query 11 schema
+struct<>
+-- !query 11 output
+
--
GitLab