Skip to content

Conversation

BuonOmo
Copy link
Collaborator

@BuonOmo BuonOmo commented Aug 18, 2025

We used to have a regex that removed the first and last single quotes from a comment. There is no information about this decision, nor any related test. This might have been a difference between PostgreSQL and CockroachDB.

Since tests are passing without it, and comments are not quoted, we can remove the regex. Moreover, if a comment would start or end with a single quote, this would remove that quote, which is not desirable.

Fixes #381

We used to have a regex that removed the first and last
single quotes from a comment. There is no information
about this decision, nor any related test. This might
have been a difference between PostgreSQL and CockroachDB.

Since tests are passing without it, and comments are not
quoted, we can remove the regex. Moreover, if a comment would
start or end with a single quote, this would remove that
quote, which is not desirable.

Fixes #381
fields.reduce({}) do |a, e|
a[e[0]] = e
a
fields.to_h do |field|
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nitpick, but reading this was hurting everytime 😅

@BuonOmo
Copy link
Collaborator Author

BuonOmo commented Aug 18, 2025

@rafiss the bug was introduced in #235 which was made in november 2021. It seems like at the time we were using docker with the image cockroachdb/example-orms-builder:20200413-1918. Do you have any idea if at that time comments in CRDB were quoted?

TBH I think we can just go for it once tests will pass.

@BuonOmo BuonOmo requested a review from rafiss August 18, 2025 11:21
@BuonOmo
Copy link
Collaborator Author

BuonOmo commented Aug 18, 2025

The failing test is the flacky one we've been having for some time now. I'll be on that next thing. In the mean time I think this is not blocking for this PR.

@rafiss
Copy link
Contributor

rafiss commented Aug 18, 2025

the bug was introduced in #235 which was made in november 2021. It seems like at the time we were using docker with the image cockroachdb/example-orms-builder:20200413-1918. Do you have any idea if at that time comments in CRDB were quoted?

Thanks for tracking down the original change! Thanks to those clues, I was able to confirm what you suggested. cockroachdb/cockroach@5660a74 is the original CRDB change that adds column comments, and indeed, they always were shown in information_schema.columns as quoted strings. The quoting behavior was removed as part of cockroachdb/cockroach@0ce5deb as an unintended, though desirable, side effect of a performance improvement.

@rafiss rafiss merged commit b56dd4a into master Aug 18, 2025
8 of 10 checks passed
@BuonOmo BuonOmo deleted the fix/comment-quotes branch August 19, 2025 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removing quotes from column comments causes parsing errors
2 participants