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.

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

sql: COMMENT ON broken inside SQL batches #35581

Closed
knz opened this issue Mar 9, 2019 · 2 comments
Closed

sql: COMMENT ON broken inside SQL batches #35581

knz opened this issue Mar 9, 2019 · 2 comments
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.

Comments

@knz
Copy link
Contributor

knz commented Mar 9, 2019

For context, the COMMENT ON ... is an alias for an UPSERT INTO system.comments wrapped inside an internal executor (there is no magic).

Compare the following:

  1. correct behavior, for reference:
> CREATE TABLE t(x INT);
CREATE TABLE

> COMMENT ON TABLE t IS 'waa';
COMMENT ON TABLE

> COMMENT ON COLUMN t.x IS 'woo';
COMMENT ON COLUMN

> SELECT * FROM system.comments;
  type | object_id | sub_id | comment
+------+-----------+--------+---------+
     1 |        54 |      0 | waa
     2 |        54 |      1 | woo
(2 rows)

> DROP TABLE t;
DROP TABLE
  1. woooops, first comment erased by second!!?!
root@127.0.0.1:25632/defaultdb> CREATE TABLE t(x INT);
CREATE TABLE

>  COMMENT ON TABLE t IS 'waa'; COMMENT ON COLUMN t.x IS 'woo';  -- two comments in one batch
COMMENT ON COLUMN

> SELECT * FROM system.comments;
  type | object_id | sub_id | comment
+------+-----------+--------+---------+
     1 |        55 |      0 | woo
     2 |        55 |      1 | woo
(2 rows)

> DROP TABLE t;
DROP TABLE
  1. bigger woops, both comments missing!?!
> CREATE TABLE t(x INT);
CREATE TABLE

>  COMMENT ON TABLE t IS 'waa'; COMMENT ON COLUMN t.x IS 'woo'; SELECT * FROM system.comments; -- upsert+select in 1 batch
  type | object_id | sub_id | comment
+------+-----------+--------+---------+
     1 |        55 |      0 | .
     2 |        55 |      1 | .
(2 rows)

> DROP TABLE t;
DROP TABLE
@knz knz added A-schema-descriptors Relating to SQL table/db descriptor handling. S-0-visible-logical-error Database stores inconsistent data in some cases, or queries return invalid results silently. A-sql-executor SQL txn logic labels Mar 9, 2019
@knz
Copy link
Contributor Author

knz commented Mar 9, 2019

If I manually replace COMMENT ON by UPSERT, the behavior is fine. This points in the direction of an error in the implementation of ResolveMutableTableDescriptor or perhaps for queries routed through the internal executor.

@knz
Copy link
Contributor Author

knz commented Mar 9, 2019

If I replace the call to ResolveMutableTableDescriptor by ResolveUncachedTableDescriptor, the error persists. This means the internal executor is erroneously dropping queries on the floor.

@knz knz changed the title sql: regression: transaction atomicity/isolation is broken inside SQL batches sql: regression: transaction atomicity/isolation is broken when using internal executor inside SQL batches Mar 9, 2019
@knz knz added S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases. and removed S-0-visible-logical-error Database stores inconsistent data in some cases, or queries return invalid results silently. labels Mar 9, 2019
@knz knz changed the title sql: regression: transaction atomicity/isolation is broken when using internal executor inside SQL batches sql: COMMENT ON broken inside SQL batches Mar 9, 2019
@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. and removed A-schema-descriptors Relating to SQL table/db descriptor handling. A-sql-executor SQL txn logic labels Mar 9, 2019
@knz knz self-assigned this Mar 9, 2019
craig bot pushed a commit that referenced this issue Mar 9, 2019
34764: sql: provide table comments for virtual tables r=knz a=hueypark

Fixes #32964
Fixes #35581

Release note (sql change): CockroachDB now provides usable comments
with optional documentation URLs for the virtual tables in
`pg_catalog`, `information_schema` and `crdb_internal`. Use `SHOW
TABLES [FROM ...] WITH COMMENT` to read. Note that `crdb_internal`
tables remain an experimental feature subject to change without
notice.

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
@craig craig bot closed this as completed in #34764 Mar 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-3-erroneous-edge-case Database produces or stores erroneous data without visible error/warning, in rare edge cases.
Projects
None yet
Development

No branches or pull requests

1 participant