Skip to content

[SPARK-21119][SQL] unset table properties should keep the table comment #18325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cloud-fan wants to merge 1 commit into from
Closed

[SPARK-21119][SQL] unset table properties should keep the table comment #18325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,14 +264,14 @@ case class AlterTableUnsetPropertiesCommand(
DDLUtils.verifyAlterTableType(catalog, table, isView)
if (!ifExists) {
propKeys.foreach { k =>
if (!table.properties.contains(k)) {
if (!table.properties.contains(k) && k != "comment") {
throw new AnalysisException(
s"Attempted to unset non-existent property '$k' in table '${table.identifier}'")
}
}
}
Copy link
Member

@viirya viirya Jun 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In above code not shown in this diff, we check if a prop key exists in table.properties or not when ifExists is false. As comment is not in table.properties, I think it will cause AnalysisException when unset comment?

// 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 tableComment = if (propKeys.contains("comment")) None else table.comment
Copy link
Member

@viirya viirya Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work for Hive table? When CatalogTable's comment is None, we will ignore it when converting it to Hive table metadata. So seems it will not be unset?

spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala

Line 935 in 9d0db5a

table.comment.foreach { c => hiveTable.setProperty("comment", c) }

Copy link
Member

@viirya viirya Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. I see. It goes to override the original table metadata if no comment is set.

val newProperties = table.properties.filter { case (k, _) => !propKeys.contains(k) }
val newTable = table.copy(properties = newProperties, comment = tableComment)
catalog.alterTable(newTable)
Expand Down
8 changes: 8 additions & 0 deletions sql/core/src/test/resources/sql-tests/inputs/describe.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ DESC FORMATTED t;

DESC EXTENDED t;

ALTER TABLE t UNSET TBLPROPERTIES (e);

DESC EXTENDED t;

ALTER TABLE t UNSET TBLPROPERTIES (comment);

DESC EXTENDED t;

DESC t PARTITION (c='Us', d=1);

DESC EXTENDED t PARTITION (c='Us', d=1);
Expand Down
201 changes: 138 additions & 63 deletions sql/core/src/test/resources/sql-tests/results/describe.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
-- Automatically generated by SQLQueryTestSuite
-- Number of queries: 32
-- Number of queries: 36


-- !query 0
Expand Down Expand Up @@ -166,10 +166,85 @@ Partition Provider Catalog


-- !query 11
DESC t PARTITION (c='Us', d=1)
ALTER TABLE t UNSET TBLPROPERTIES (e)
-- !query 11 schema
struct<col_name:string,data_type:string,comment:string>
struct<>
-- !query 11 output



-- !query 12
DESC EXTENDED t
-- !query 12 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 12 output
a string
b int
c string
d string
# Partition Information
# col_name data_type comment
c string
d string

# Detailed Table Information
Database default
Table t
Created [not included in comparison]
Last Access [not included in comparison]
Type MANAGED
Provider parquet
Num Buckets 2
Bucket Columns [`a`]
Sort Columns [`b`]
Comment table_comment
Location [not included in comparison]sql/core/spark-warehouse/t
Storage Properties [a=1, b=2]
Partition Provider Catalog


-- !query 13
ALTER TABLE t UNSET TBLPROPERTIES (comment)
-- !query 13 schema
struct<>
-- !query 13 output



-- !query 14
DESC EXTENDED t
-- !query 14 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 14 output
a string
b int
c string
d string
# Partition Information
# col_name data_type comment
c string
d string

# Detailed Table Information
Database default
Table t
Created [not included in comparison]
Last Access [not included in comparison]
Type MANAGED
Provider parquet
Num Buckets 2
Bucket Columns [`a`]
Sort Columns [`b`]
Location [not included in comparison]sql/core/spark-warehouse/t
Storage Properties [a=1, b=2]
Partition Provider Catalog


-- !query 15
DESC t PARTITION (c='Us', d=1)
-- !query 15 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 15 output
a string
b int
c string
Expand All @@ -180,11 +255,11 @@ c string
d string


-- !query 12
-- !query 16
DESC EXTENDED t PARTITION (c='Us', d=1)
-- !query 12 schema
-- !query 16 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 12 output
-- !query 16 output
a string
b int
c string
Expand All @@ -209,11 +284,11 @@ Location [not included in comparison]sql/core/spark-warehouse/t
Storage Properties [a=1, b=2]


-- !query 13
-- !query 17
DESC FORMATTED t PARTITION (c='Us', d=1)
-- !query 13 schema
-- !query 17 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 13 output
-- !query 17 output
a string
b int
c string
Expand All @@ -238,31 +313,31 @@ Location [not included in comparison]sql/core/spark-warehouse/t
Storage Properties [a=1, b=2]


-- !query 14
-- !query 18
DESC t PARTITION (c='Us', d=2)
-- !query 14 schema
-- !query 18 schema
struct<>
-- !query 14 output
-- !query 18 output
org.apache.spark.sql.catalyst.analysis.NoSuchPartitionException
Partition not found in table 't' database 'default':
c -> Us
d -> 2;


-- !query 15
-- !query 19
DESC t PARTITION (c='Us')
-- !query 15 schema
-- !query 19 schema
struct<>
-- !query 15 output
-- !query 19 output
org.apache.spark.sql.AnalysisException
Partition spec is invalid. The spec (c) must match the partition spec (c, d) defined in table '`default`.`t`';


-- !query 16
-- !query 20
DESC t PARTITION (c='Us', d)
-- !query 16 schema
-- !query 20 schema
struct<>
-- !query 16 output
-- !query 20 output
org.apache.spark.sql.catalyst.parser.ParseException

PARTITION specification is incomplete: `d`(line 1, pos 0)
Expand All @@ -272,55 +347,55 @@ DESC t PARTITION (c='Us', d)
^^^


-- !query 17
-- !query 21
DESC temp_v
-- !query 17 schema
-- !query 21 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 17 output
-- !query 21 output
a string
b int
c string
d string


-- !query 18
-- !query 22
DESC TABLE temp_v
-- !query 18 schema
-- !query 22 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 18 output
-- !query 22 output
a string
b int
c string
d string


-- !query 19
-- !query 23
DESC FORMATTED temp_v
-- !query 19 schema
-- !query 23 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 19 output
-- !query 23 output
a string
b int
c string
d string


-- !query 20
-- !query 24
DESC EXTENDED temp_v
-- !query 20 schema
-- !query 24 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 20 output
-- !query 24 output
a string
b int
c string
d string


-- !query 21
-- !query 25
DESC temp_Data_Source_View
-- !query 21 schema
-- !query 25 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 21 output
-- !query 25 output
intType int test comment test1
stringType string
dateType date
Expand All @@ -339,42 +414,42 @@ arrayType array<string>
structType struct<f1:string,f2:int>


-- !query 22
-- !query 26
DESC temp_v PARTITION (c='Us', d=1)
-- !query 22 schema
-- !query 26 schema
struct<>
-- !query 22 output
-- !query 26 output
org.apache.spark.sql.AnalysisException
DESC PARTITION is not allowed on a temporary view: temp_v;


-- !query 23
-- !query 27
DESC v
-- !query 23 schema
-- !query 27 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 23 output
-- !query 27 output
a string
b int
c string
d string


-- !query 24
-- !query 28
DESC TABLE v
-- !query 24 schema
-- !query 28 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 24 output
-- !query 28 output
a string
b int
c string
d string


-- !query 25
-- !query 29
DESC FORMATTED v
-- !query 25 schema
-- !query 29 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 25 output
-- !query 29 output
a string
b int
c string
Expand All @@ -392,11 +467,11 @@ View Query Output Columns [a, b, c, d]
Table Properties [view.query.out.col.3=d, view.query.out.col.0=a, view.query.out.numCols=4, view.default.database=default, view.query.out.col.1=b, view.query.out.col.2=c]


-- !query 26
-- !query 30
DESC EXTENDED v
-- !query 26 schema
-- !query 30 schema
struct<col_name:string,data_type:string,comment:string>
-- !query 26 output
-- !query 30 output
a string
b int
c string
Expand All @@ -414,42 +489,42 @@ View Query Output Columns [a, b, c, d]
Table Properties [view.query.out.col.3=d, view.query.out.col.0=a, view.query.out.numCols=4, view.default.database=default, view.query.out.col.1=b, view.query.out.col.2=c]


-- !query 27
-- !query 31
DESC v PARTITION (c='Us', d=1)
-- !query 27 schema
-- !query 31 schema
struct<>
-- !query 27 output
-- !query 31 output
org.apache.spark.sql.AnalysisException
DESC PARTITION is not allowed on a view: v;


-- !query 28
-- !query 32
DROP TABLE t
-- !query 28 schema
-- !query 32 schema
struct<>
-- !query 28 output
-- !query 32 output



-- !query 29
-- !query 33
DROP VIEW temp_v
-- !query 29 schema
-- !query 33 schema
struct<>
-- !query 29 output
-- !query 33 output



-- !query 30
-- !query 34
DROP VIEW temp_Data_Source_View
-- !query 30 schema
-- !query 34 schema
struct<>
-- !query 30 output
-- !query 34 output



-- !query 31
-- !query 35
DROP VIEW v
-- !query 31 schema
-- !query 35 schema
struct<>
-- !query 31 output
-- !query 35 output