-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-23525] [SQL] Support ALTER TABLE CHANGE COLUMN COMMENT for external hive table #20696
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.
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
Conversation
| // Ensure that change column will preserve other metadata fields. | ||
| sql("ALTER TABLE dbx.tab1 CHANGE COLUMN col1 col1 INT COMMENT 'this is col1'") | ||
| assert(getMetadata("col1").getString("key") == "value") | ||
| assert(getMetadata("col1").getString("comment") == "this is col1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't verify the comment is correctly set to the column metadata, and that caused the issue. :(
| // Find the origin column from schema by column name. | ||
| val originColumn = findColumnByName(table.schema, columnName, resolver) | ||
| // Find the origin column from dataSchema by column name. | ||
| val originColumn = findColumnByName(table.dataSchema, columnName, resolver) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only data schema? How about partitioning schema?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As described in the comment, we only support altering non-partition columns:
A command to change the column for a table, only support changing the comment of a non-partition
column for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a negative test case to cover that?
|
Test build #87786 has finished for PR 20696 at commit
|
|
Test build #87842 has finished for PR 20696 at commit
|
| -- !query 30 output | ||
|
|
||
| org.apache.spark.sql.AnalysisException | ||
| Invalid column reference 'c', table data schema is 'StructType(StructField(a,IntegerType,true), StructField(b,StringType,true))'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a better error message here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about
Can't find column `c` given table data columns [`a`, `b`]
|
LGTM |
|
Test build #88031 has finished for PR 20696 at commit
|
|
thanks, merging to master/2.3! |
…rnal hive table ## What changes were proposed in this pull request? The following query doesn't work as expected: ``` CREATE EXTERNAL TABLE ext_table(a STRING, b INT, c STRING) PARTITIONED BY (d STRING) LOCATION 'sql/core/spark-warehouse/ext_table'; ALTER TABLE ext_table CHANGE a a STRING COMMENT "new comment"; DESC ext_table; ``` The comment of column `a` is not updated, that's because `HiveExternalCatalog.doAlterTable` ignores table schema changes. To fix the issue, we should call `doAlterTableDataSchema` instead of `doAlterTable`. ## How was this patch tested? Updated `DDLSuite.testChangeColumn`. Author: Xingbo Jiang <[email protected]> Closes #20696 from jiangxb1987/alterColumnComment. (cherry picked from commit ac76eff) Signed-off-by: Wenchen Fan <[email protected]>
…rnal hive table ## What changes were proposed in this pull request? The following query doesn't work as expected: ``` CREATE EXTERNAL TABLE ext_table(a STRING, b INT, c STRING) PARTITIONED BY (d STRING) LOCATION 'sql/core/spark-warehouse/ext_table'; ALTER TABLE ext_table CHANGE a a STRING COMMENT "new comment"; DESC ext_table; ``` The comment of column `a` is not updated, that's because `HiveExternalCatalog.doAlterTable` ignores table schema changes. To fix the issue, we should call `doAlterTableDataSchema` instead of `doAlterTable`. ## How was this patch tested? Updated `DDLSuite.testChangeColumn`. Author: Xingbo Jiang <[email protected]> Closes apache#20696 from jiangxb1987/alterColumnComment.
…rnal hive table ## What changes were proposed in this pull request? The following query doesn't work as expected: ``` CREATE EXTERNAL TABLE ext_table(a STRING, b INT, c STRING) PARTITIONED BY (d STRING) LOCATION 'sql/core/spark-warehouse/ext_table'; ALTER TABLE ext_table CHANGE a a STRING COMMENT "new comment"; DESC ext_table; ``` The comment of column `a` is not updated, that's because `HiveExternalCatalog.doAlterTable` ignores table schema changes. To fix the issue, we should call `doAlterTableDataSchema` instead of `doAlterTable`. ## How was this patch tested? Updated `DDLSuite.testChangeColumn`. Author: Xingbo Jiang <[email protected]> Closes apache#20696 from jiangxb1987/alterColumnComment.
…rnal hive table ## What changes were proposed in this pull request? The following query doesn't work as expected: ``` CREATE EXTERNAL TABLE ext_table(a STRING, b INT, c STRING) PARTITIONED BY (d STRING) LOCATION 'sql/core/spark-warehouse/ext_table'; ALTER TABLE ext_table CHANGE a a STRING COMMENT "new comment"; DESC ext_table; ``` The comment of column `a` is not updated, that's because `HiveExternalCatalog.doAlterTable` ignores table schema changes. To fix the issue, we should call `doAlterTableDataSchema` instead of `doAlterTable`. ## How was this patch tested? Updated `DDLSuite.testChangeColumn`. Author: Xingbo Jiang <[email protected]> Closes apache#20696 from jiangxb1987/alterColumnComment. (cherry picked from commit ac76eff) Signed-off-by: Wenchen Fan <[email protected]>
What changes were proposed in this pull request?
The following query doesn't work as expected:
The comment of column
ais not updated, that's becauseHiveExternalCatalog.doAlterTableignores table schema changes. To fix the issue, we should calldoAlterTableDataSchemainstead ofdoAlterTable.How was this patch tested?
Updated
DDLSuite.testChangeColumn.