Skip to content

eval: remove wrapping from builtin errors #147195

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

Merged
merged 3 commits into from
May 27, 2025

Conversation

yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented May 23, 2025

roachtest/pg_regress: accept recent diff

Fixes: #147252.

eval: remove wrapping from builtin errors

This PR removes the code to perform error wrapping around the errors produced by the builtin function which provides a bit more context (like the function name). This wrapping dates all the way back to a commit from 2015 716837e. Over the years it was expanded to also provide some more context to the sentry reports (7b703de). I don't think either of these provide much value since it's usually clear which builtin the error corresponds to and having just the builtin name in the sentry report doesn't make it any more actionable. Removing of this logic allows us to match errors as they are produced by Postgres.

builins: unify an error message in substring with PG

Epic: None
Release note: None

Copy link

blathers-crl bot commented May 23, 2025

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@yuzefovich yuzefovich force-pushed the pg_regress_builtins branch from db6f19f to bf02be1 Compare May 23, 2025 17:11
@yuzefovich yuzefovich marked this pull request as ready for review May 23, 2025 17:12
@yuzefovich yuzefovich requested review from a team as code owners May 23, 2025 17:12
@yuzefovich yuzefovich requested review from kyle-a-wong, arjunmahishi, aa-joshi, Abhinav1299, herkolategan, golgeek, kev-cao and rytaft and removed request for a team May 23, 2025 17:12
@yuzefovich yuzefovich requested review from a team, kev-cao, mgartner and DrewKimball and removed request for kyle-a-wong, arjunmahishi, aa-joshi, Abhinav1299, herkolategan, golgeek, kev-cao, rytaft, a team and mgartner May 23, 2025 17:13
@yuzefovich
Copy link
Member Author

Commits 2 through 5 will be squashed into a single one, but I kept them separate for ease of reviewing.

I looked into this because I left a TODO for myself to remove this wrapping context. That said, I don't feel too strongly about doing this, so if people find the current behavior preferable, I'm fine not merging this PR and simply removing the TODO.

Copy link
Collaborator

@DrewKimball DrewKimball left a comment

Choose a reason for hiding this comment

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

:lgtm: It's nice to see so many diffs go away!

Reviewed 4 of 4 files at r1, 3 of 3 files at r2, 46 of 46 files at r3, 2 of 2 files at r4, 31 of 31 files at r5, 8 of 8 files at r6, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @yuzefovich)

This commit removes the code to perform error wrapping around the errors
produced by the builtin function which provides a bit more context (like
the function name). This wrapping dates all the way back to a commit
from 2015 716837e. Over the years it
was expanded to also provide some more context to the sentry reports
(7b703de). I don't think either of
these provide much value since it's usually clear which builtin the
error corresponds to and having just the builtin name in the sentry
report doesn't make it any more actionable. Removing of this logic
allows us to match errors as they are produced by Postgres.

Release note: None
@yuzefovich yuzefovich force-pushed the pg_regress_builtins branch from bf02be1 to 93bd46c Compare May 27, 2025 17:08
@yuzefovich
Copy link
Member Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented May 27, 2025

@craig craig bot merged commit bbe6bc6 into cockroachdb:master May 27, 2025
22 checks passed
@yuzefovich yuzefovich deleted the pg_regress_builtins branch May 27, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roachtest: pg_regress failed
3 participants