sql: print comments in SHOW CREATE TABLE#43152
Conversation
|
This change is |
|
Will this get into v.19.2.3? I'd quite like to deploy this, so that I can start writing comments without them getting lost. |
d8a83c0 to
8288533
Compare
|
@jordanlewis Would you review this? |
knz
left a comment
There was a problem hiding this comment.
@namibj no this will not get in 19.2.x. We do not add or changes features in release branches unless they are security-critical. The earliest this will be released will be v20.1.
@hueypark Jordan is away and he asked me to review.
As usual, your work here is very good. I have just one suggestion to improve the interface and some stylistics edits on code comments.
A request on the commit message: can you expand the release note to explain what is happening, for example:
Release note (sql change): SHOW CREATE TABLE now also emits the COMMENT statements sufficient to populate the table's user-defined comments, if any, alongside the CREATE statement proper.
Finally, could you please file an issue for supporting COMMENT ON VIEW and COMMENT ON SEQUENCE for completeness, and mention in the issue description that the code for SHOW CREATE must be adapted accordingly. Thank you.
Reviewed 8 of 8 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @hueypark)
pkg/sql/show_create.go, line 48 at r1 (raw file):
// the prefix when the given table references other tables in the // current database. func ShowCreateTable(
I would make this function a method on *planner, then add its in terface to the PlanHookState interface.
pkg/sql/show_create_clauses.go, line 26 at r1 (raw file):
) // tableComment has comment data for table.
-
tableConmment->tableComments -
// tableComments stores the comment data for a table.
pkg/sql/show_create_clauses.go, line 38 at r1 (raw file):
} // selectComment returns table comment.
Detail what is being done here:
// selectComment retrieves all the comments pertaining to a table (comments on the table
// itself but also column and index comments.)pkg/sql/show_create_clauses.go, line 94 at r1 (raw file):
} // showComment writes a valid SQL representation of a COMMENT clause.
-
showComment->showComments -
// showComments prints out the COMMENT statement sufficient to populate a table's comments, including its index and column comments.
|
sorry regarding my last comment of course the word should be plural: |
Fixes cockroachdb#42875 Release note (sql change): SHOW CREATE TABLE now also emits the COMMENT statements sufficient to populate the table's user-defined comments, if any, alongside the CREATE statement proper.
8288533 to
08be7c4
Compare
hueypark
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/show_create.go, line 48 at r1 (raw file):
Previously, knz (kena) wrote…
I would make this function a method on
*planner, then add its in terface to thePlanHookStateinterface.
Done.
pkg/sql/show_create_clauses.go, line 26 at r1 (raw file):
Previously, knz (kena) wrote…
tableConmment->tableComments
// tableComments stores the comment data for a table.
Done.
pkg/sql/show_create_clauses.go, line 38 at r1 (raw file):
Previously, knz (kena) wrote…
Detail what is being done here:
// selectComment retrieves all the comments pertaining to a table (comments on the table // itself but also column and index comments.)
Done.
pkg/sql/show_create_clauses.go, line 94 at r1 (raw file):
Previously, knz (kena) wrote…
showComment->showComments
// showComments prints out the COMMENT statement sufficient to populate a table's comments, including its index and column comments.
Done.
knz
left a comment
There was a problem hiding this comment.
Very nice. Thank you!
Reviewed 4 of 4 files at r2.
Reviewable status:
hueypark
left a comment
There was a problem hiding this comment.
I added an issue for VIEW and SEQUENCE(#44135).
Reviewable status:
|
Thank you again for your contribution. bors r+ |
|
Thank you for the review! |
Build succeeded |
Fixes #42875
Release note (sql change): SHOW CREATE TABLE now prints comments.