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

cli/dump: COMMENT ON missing #42875

Closed
namibj opened this issue Dec 1, 2019 · 5 comments · Fixed by #43152
Closed

cli/dump: COMMENT ON missing #42875

namibj opened this issue Dec 1, 2019 · 5 comments · Fixed by #43152
Labels
A-sql-vtables Virtual tables - pg_catalog, information_schema etc C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@namibj
Copy link

namibj commented Dec 1, 2019

Describe the problem

Please describe the issue you observed, and any steps we can take to reproduce it:
If a column has a comment (via COMMENT ON), the comment will be missing in the cockroach dumo --dump-mode schema output.

To Reproduce

What did you do? Describe in your own words.

If possible, provide steps to reproduce the behavior:

  1. Create a schema with a comment (the example in the docs will do)
  2. Issue cockroach dump --dump-mode schema <database> | grep COMMENT
  3. Observe the lack of output, which implies no comment was dumped with the schema.

Expected behavior
The schema is dumped with all comments.

Environment:

  • CockroachDB version v19.2.0
  • Server OS: Linux/archlinux
  • Client app cockroach dump

Additional context
What was the impact?

It silently swallowed my comments, which specified preliminary enum mappings for int2 columns. Thankfully it wasn't much.
@jordanlewis jordanlewis added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Dec 2, 2019
@jordanlewis
Copy link
Member

Thanks for the report!

@hueypark could I send this one your way, if you have some time?

cc @dt - dump counts as BulkIO, right?

@hueypark
Copy link
Contributor

hueypark commented Dec 2, 2019

@jordanlewis Of course. It's likely that I will be able to create the first PR on December 15th. It seems appropriate to be implemented this on crdb_internal.create_statements in crdb_internal.go. Is that correct?

@namibj
Copy link
Author

namibj commented Dec 3, 2019

@hueypark It certainly looks like it.

@dt
Copy link
Member

dt commented Dec 3, 2019 •
edited
Loading

This reminds me that this is not the first time we've discovered a new SQL feature that was never added to SHOW CREATE TABLE when it was introduced, and the current setup is prone to such mistakes. Filed #42917 to try to address this testing gap.

@namibj
Copy link
Author

namibj commented Dec 4, 2019

@dt this sounds almost like fuzzing. If you can decouple it enough from the transactional layers, you might be able to fuzz directly for round-trip idempotence. Thnaks for adressing this.

@hueypark hueypark mentioned this issue Dec 14, 2019
Merged
hueypark added a commit to hueypark/cockroach that referenced this issue Jan 20, 2020
@hueypark
sql: print comments in SHOW CREATE TABLE
8288533 
Fixes cockroachdb#42875

Release note (sql change): SHOW CREATE TABLE now prints comments.
@knz knz added A-sql-vtables Virtual tables - pg_catalog, information_schema etc C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. and removed C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Jan 20, 2020
@hueypark hueypark mentioned this issue Jan 20, 2020
Open
4 tasks
craig bot pushed a commit that referenced this issue Jan 20, 2020
@hueypark
Merge #43152
8cf3adf 
43152: sql: print comments in SHOW CREATE TABLE r=knz a=hueypark

Fixes #42875

Release note (sql change): SHOW CREATE TABLE now prints comments.

Co-authored-by: Jaewan Park <[email protected]>
@craig craig bot closed this as completed in 08be7c4 Jan 20, 2020
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-vtables Virtual tables - pg_catalog, information_schema etc C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: print comments in SHOW CREATE TABLE hueypark/cockroach
5 participants
@dt @jordanlewis @knz @namibj @hueypark