Skip to content
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

Jump to bottom

sql: Support 'COMMENT ON ...' for tables, columns and other objects #19472

Closed
3 of 4 tasks
flokli opened this issue Oct 24, 2017 · 10 comments
Closed
3 of 4 tasks

sql: Support 'COMMENT ON ...' for tables, columns and other objects #19472

flokli opened this issue Oct 24, 2017 · 10 comments
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) X-anchored-telemetry The issue number is anchored by telemetry references.

Comments

@flokli
Copy link

flokli commented Oct 24, 2017 •
edited by knz
Loading

Postgres docs on COMMENT ON: https://www.postgresql.org/docs/current/static/sql-comment.html
@dt dt added A-sql-pgcompat Semantic compatibility with PostgreSQL good first issue labels Oct 24, 2017
@dt
Copy link
Member

dt commented Oct 24, 2017 •
edited
Loading

Huh, interesting. Thanks for isolating the issue and including an example of it used in the wild!

Indeed, cockroach doesn't currently support that syntax, or indeed store any sort of table or column comments or plain-text descriptions.

@knz it seems like it this shouldn't be too hard to add and may improve some off-the-shelf ORM/app compatibility (based on that example). Seems like a decent SQL starter project?

@knz
Copy link
Contributor

knz commented Oct 24, 2017

Yep!

@dt dt added the help wanted Help is requested / needed by the one who filed the issue to fix it. label Oct 24, 2017
@lgo lgo mentioned this issue Dec 20, 2017
Closed
10 tasks
@ZacharyThomas
Copy link

Going to try and look at this if nobody else is. PG lets you comment on nearly everything. Would we want to adopt that approach here, or should I focus only on tables?

It also looks like PG stores this in two new tables by oid. It seems a little more complicated than I initially would have thought but I'll give it my best shot. Do ya'll see any obvious gotchas with that approach?

@knz
Copy link
Contributor

knz commented Dec 27, 2017

Hi @ZacharyThomas !
Thank you for wanting to help here.
In the initial implementation we recommend that you modify CockroachDB to recognize the comment syntax but just ignore it and not store the comments anywhere.
Storing the comments and providing an interface to retrieve them would be a much significant undertaking and I wouldn't recommend it as a starter project.

@ZacharyThomas ZacharyThomas mentioned this issue Dec 27, 2017
Merged
@knz knz mentioned this issue Jan 30, 2018
Open
@knz
Copy link
Contributor

knz commented Jan 30, 2018

So thanks to #21063 CockroachDB 2.0 will have minimal support for this syntax. @flokli it would be great if you could indicate whether this is sufficient to enable compatibility with Drupal. Otherwise, please create new issue(s) as appropriate and link them to #22211.
Thank you!

@knz knz closed this as completed Jan 30, 2018
@flokli flokli mentioned this issue Feb 6, 2018
Closed
@awoods187 awoods187 mentioned this issue Oct 29, 2018
Closed
4 tasks
@awoods187
Copy link
Contributor

It looks like this is actually not supported yet so I'm reopening it

@awoods187 awoods187 reopened this Oct 30, 2018
@knz knz mentioned this issue Nov 12, 2018
Open
@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Nov 12, 2018
@BramGruneir
Copy link
Member

Let's not forget that there are a ton of objects that can have comments on them:
https://www.postgresql.org/docs/9.1/sql-comment.html

Let's make sure that any work towards this is extensible.

Also, we should populate this and any other internal tables: https://www.postgresql.org/docs/9.6/catalog-pg-description.html

hueypark added a commit to hueypark/cockroach that referenced this issue Nov 17, 2018
@hueypark
sql: implement COMMENT ON TABLE
b26e64f 
See cockroachdb#19472

Release note (sql change): support COMMENT ON TABLE syntax
@hueypark hueypark mentioned this issue Nov 17, 2018
Merged
@bithavoc
Copy link

@hueypark are you going to work on the system.comments approach?

@knz knz added the X-anchored-telemetry The issue number is anchored by telemetry references. label Nov 22, 2018
hueypark added a commit to hueypark/cockroach that referenced this issue Nov 24, 2018
@hueypark
sql: implement COMMENT ON TABLE
672b4e8 
See cockroachdb#19472

Release note (sql change): support COMMENT ON TABLE syntax
@knz
Copy link
Contributor

knz commented Nov 26, 2018

@bithavoc yes it is being done in a new update to #32442.

hueypark added a commit to hueypark/cockroach that referenced this issue Dec 2, 2018
@hueypark
sql: support COMMENT ON TABLE
afd5aab 
Create system.comments table for comment. Each column description:
    - type: type of object, to distinguish between db, table, column and others(https://www.postgresql.org/docs/9.1/sql-comment.html)
    - object_id: object ID, this will be usually db/table desc ID
    - sub_id: sub ID for columns inside table, 0 for pure table
    - comment: the comment
And system.comments has public priviliege because this table used in SHOW TABLES.

See cockroachdb#19472

Release note (sql change): support COMMENT ON TABLE syntax
hueypark added a commit to hueypark/cockroach that referenced this issue Dec 2, 2018
@hueypark
sql: support COMMENT ON TABLE
1ecedf2 
Create system.comments table for comment. Each column description:
    - type: type of object, to distinguish between db, table, column and others(https://www.postgresql.org/docs/9.1/sql-comment.html)
    - object_id: object ID, this will be usually db/table desc ID
    - sub_id: sub ID for columns inside table, 0 for pure table
    - comment: the comment
And system.comments has public priviliege because this table used in SHOW TABLES.

See cockroachdb#19472

Release note (sql change): support COMMENT ON TABLE syntax
hueypark added a commit to hueypark/cockroach that referenced this issue Dec 7, 2018
@hueypark
sql: support COMMENT ON TABLE
89ee8c2 
Create system.comments table for comment. Each column description:
    - type: type of object, to distinguish between db, table, column and others(https://www.postgresql.org/docs/9.1/sql-comment.html)
    - object_id: object ID, this will be usually db/table desc ID
    - sub_id: sub ID for columns inside table, 0 for pure table
    - comment: the comment
And system.comments has public privilege because this table used in SHOW TABLES.

See cockroachdb#19472

Release note (sql change): support `COMMENT ON TABLE` syntax, support `WITH COMMENT` option for `SHOW TABLE`
@knz knz changed the title sql: Support 'COMMENT ON TABLE foo is 'bar' sql: Support 'COMMENT ON ...' for tables, columns and other objects Dec 9, 2018
This was referenced Dec 9, 2018
Closed
Closed
Closed
craig bot pushed a commit that referenced this issue Dec 9, 2018
@hueypark
Merge #32442
783767c 
32442: sql: implement `COMMENT ON TABLE` r=knz a=hueypark

Informs  #19472.

This patch introduces support for table comments.

The syntax to set or delete a comment is the same as postgres:
`COMMENT ON TABLE ... IS ...`. See:
postgresql.org/docs/9.1/sql-comment.html

This also makes `pg_catalog.pg_description` and `obj_description()` do
the right thing for compatibility with 3rd party clients.

This is supported by a new system table `system.comments`, which is
extensible to support comments on other database objects than tables:

- its `type` column indicates the type of object, to distinguish
  between db, table, column and others. For now just one type is
  defined.
- `object_id`: table or database ID, relative to the `type`.
- `sub_id`: when a comment is placed on an object "inside" another, eg
  a column inside a table.
- `comment`: the comment proper.

This design of `system.comments` mimics pg's own `pg_description`
which uses the same schema.

Release note (sql change): CockroachDB now supports associating
comments to SQL tables using PostgreSQL's `COMMENT ON TABLE`
syntax. This also provides proper support for pg's
`pg_catalog.pg_description` and built-in function `obj_description()`.

Release note (sql change): The `SHOW TABLES` statement
now supports printing out table comments using the optional phrase
`WITH COMMENT`, e.g `SHOW TABLES FROM mydb WITH COMMENT`.

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
@knz knz removed good first issue help wanted Help is requested / needed by the one who filed the issue to fix it. labels Dec 9, 2018
craig bot pushed a commit that referenced this issue Dec 10, 2018
@hueypark
Merge #32442
8f30db0 
32442: sql: implement `COMMENT ON TABLE` r=knz a=hueypark

Informs  #19472.

This patch introduces support for table comments.

The syntax to set or delete a comment is the same as postgres:
`COMMENT ON TABLE ... IS ...`. See:
https://postgresql.org/docs/9.1/sql-comment.html

This also makes `pg_catalog.pg_description` and `obj_description()` do
the right thing for compatibility with 3rd party clients.

This is supported by a new system table `system.comments`, which is
extensible to support comments on other database objects than tables:

- its `type` column indicates the type of object, to distinguish
  between db, table, column and others. For now just one type is
  defined.
- `object_id`: table or database ID, relative to the `type`.
- `sub_id`: when a comment is placed on an object "inside" another, eg
  a column inside a table.
- `comment`: the comment proper.

This design of `system.comments` mimics pg's own `pg_description`
which uses the same schema.

Release note (sql change): CockroachDB now supports associating
comments to SQL tables using PostgreSQL's `COMMENT ON TABLE`
syntax. This also provides proper support for pg's
`pg_catalog.pg_description` and built-in function `obj_description()`.

Release note (sql change): The `SHOW TABLES` statement
now supports printing out table comments using the optional phrase
`WITH COMMENT`, e.g `SHOW TABLES FROM mydb WITH COMMENT`.

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
@hueypark hueypark mentioned this issue Dec 12, 2018
Merged
@hueypark hueypark mentioned this issue Dec 25, 2018
Merged
craig bot pushed a commit that referenced this issue Dec 30, 2018
@hueypark
Merge #33057
6dac732 
33057: sql: support `COMMENT ON DATABASE` r=knz a=hueypark

Informs #19472.

This patch introduces support for database comments.

The syntax to set or delete a comment is the same as postgres:
`COMMENT ON DATABASE ... IS ...`. See:
https://www.postgresql.org/docs/9.1/sql-comment.html

Release note (sql change): CockroachDB now supports associating
comments to SQL databases using PostgreSQL's `COMMENT ON DATABASE`
syntax. This also provides proper support for pg's
`pg_catalog.pg_description` and built-in function `obj_description()`.

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
craig bot pushed a commit that referenced this issue Dec 31, 2018
@hueypark
Merge #33355
a8afcab 
33355: sql: support `COMMENT ON COLUMN` r=knz a=hueypark

Informs #19472.

This patch introduces support for table column comments.

The syntax to set or delete a comment is the same as postgres:
`COMMENT ON COLUMN ... IS ...`. See:
https://www.postgresql.org/docs/9.1/sql-comment.html

Release note (sql change): CockroachDB now supports associating
comments to SQL table column using PostgreSQL's `COMMENT ON COLIMN`
syntax. This also provides proper support for pg's
`pg_catalog.pg_description` and built-in function `col_description()`.

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
@jordanlewis
Copy link
Member

This is done but for schemas which we don't support anyway - closing. Thanks again @hueypark.

arulajmani added a commit to arulajmani/cockroach that referenced this issue Sep 20, 2019
@arulajmani
sql: remove unimplemented error message from COMMENT ON statement
7979cf1 
Any syntax error while using the COMMENT ON statement returns an error
referencing cockroachdb#19472. With cockroachdb#19472 closed, we should no longer reference
it when the syntax is wrong.

Fixes cockroachdb#40713

Release note: None

Release Justification: Very minor change
@arulajmani arulajmani mentioned this issue Sep 20, 2019
Merged
craig bot pushed a commit that referenced this issue Sep 20, 2019
@arulajmani @jordanlewis
Merge #40943 #40944
1adb5d7 
40943: sql: remove unimplemented error message from COMMENT ON statement r=arulajmani a=arulajmani

Any syntax error while using the COMMENT ON statement returns an error
referencing #19472. With #19472 closed, we should no longer reference
it when the syntax is wrong.

Fixes #40713

Release note: None

Release Justification: Very minor change

40944: roachtest: tpcc-max had too low of a timeout r=jordanlewis a=jordanlewis

It's supposed to run for 6 days, but used the default timeout of 10
hours.

Closes #38558.

Release note: None
Release justification: test-only change

Co-authored-by: Arul Ajmani <arula@cockroachlabs.com>
Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
@13rac1 13rac1 mentioned this issue Jul 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) X-anchored-telemetry The issue number is anchored by telemetry references.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@dt @jordanlewis @flokli @bithavoc @knz @BramGruneir @ZacharyThomas @awoods187