-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-14518] [SQL] Support Comment in CREATE VIEW #12288
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
|
Test build #55481 has finished for PR 12288 at commit
|
|
cc @yhuai @andrewor14 Thanks! |
| private def createView( | ||
| ctx: ParserRuleContext, | ||
| name: TableIdentifierContext, | ||
| description: Option[String], |
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.
Should we just call it comment? Or, are we using description in other places?
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.
CatalogDatabase uses description as comment. Maybe comment is more straightforward. Let me change it. Thanks!
|
@gatorsmile Thank you for working on it. How about we hold off the change until we merge https://github.com/apache/spark/pull/12271/files? That PR is adding the field for comment in the CatalogTable. |
|
Sure, no problem. : ) |
|
Test build #55485 has finished for PR 12288 at commit
|
# Conflicts: # sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala
|
Test build #55785 has finished for PR 12288 at commit
|
|
@yhuai @andrewor14 @cloud-fan @liancheng After the merge of |
| sql(s"CREATE VIEW $viewName COMMENT 'no comment' AS SELECT * FROM $tabName") | ||
| val tableMetadata = catalog.getTableMetadata(TableIdentifier(tabName, Some("default"))) | ||
| val viewMetadata = catalog.getTableMetadata(TableIdentifier(viewName, Some("default"))) | ||
| assert(tableMetadata.properties.filter(_._1 != "transient_lastDdlTime") |
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'd like to do assert(tableMetadata.properties.get("comment") == "BLABLA")
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.
Sure, do it now. Thanks!
|
LGTM except one minor comment |
|
Test build #55816 has finished for PR 12288 at commit
|
|
Thanks. Merging to master. |
What changes were proposed in this pull request?
HQL Syntax: Create View
Add a support for the
[COMMENT view_comment]clauseHow was this patch tested?
Modified the existing test cases to verify the correctness.