pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)
Hi, hackers!
According to commit e3a87b4991cc2d00 the parentheses framing
(errcode...) macro are not necessary.
A small patch where they are removed is attached.
Respectfully,
Mikhail Litsarev, Postgres Pro.
Attachments:
v1-0001-pg_stat_statements-Remove-errcode.-framing-parent.patchtext/x-diff; name=v1-0001-pg_stat_statements-Remove-errcode.-framing-parent.patchDownload+57-58
Hi,
e3a87b499 states "There's no intention to make wholesale
changes of existing ereport calls ..."
Why do it for pg_stat_statements only when there are a bunch
of other places?
Converting one module in isolation doesn't make the new style
more widely known, so if we are going to do this, we should be
consistent. right?
--
Sami Imseih
Amazon Web Services (AWS)
more widely known, so if we are going to do this, we should be
consistent. right?
Sure! Which modules do you think should be updated? All the ones in
contrib/ folder or all the codebase?
And should it be one patch for the whole code or a bunch of patched for
each module in the conrib and in the root?
M.
I don't see why we should do this at all. My earlier point about consistency is
a nice-to-have, but I don't think it's an urgent matter that we should
change all
the files, and I don't see why we should for existing pg_stat_statements calls.
It would be up to reviewers to catch this for new (or modified) error handling
and hopefully someone familiarizes themselves with
https://www.postgresql.org/docs/current/error-message-reporting.html
Others may have a different opinion.
--
Sami
Sami Imseih <samimseih@gmail.com> writes:
e3a87b499 states "There's no intention to make wholesale
changes of existing ereport calls ..."
Why do it for pg_stat_statements only when there are a bunch
of other places?
I think we should flat out reject this patch. It makes no functional
improvement while creating a merge hazard for future back-patches.
That hazard might not be very large for just changing these few
spots, but any more-aggressive attempt at changing existing ereports'
style will certainly result in pain.
As e3a87b499 said, there was no plan to change existing code and
I think that decision should still stand.
regards, tom lane
Hi,
On Thu, Jun 25, 2026 at 1:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Sami Imseih <samimseih@gmail.com> writes:
e3a87b499 states "There's no intention to make wholesale
changes of existing ereport calls ..."Why do it for pg_stat_statements only when there are a bunch
of other places?I think we should flat out reject this patch. It makes no functional
improvement while creating a merge hazard for future back-patches.
That hazard might not be very large for just changing these few
spots, but any more-aggressive attempt at changing existing ereports'
style will certainly result in pain.As e3a87b499 said, there was no plan to change existing code and
I think that decision should still stand.
+1 to what's said above. The merge burden is real (recently it took me
more than 30 minutes just to create patches for back-branches even
though most of the code remains the same in the function being
changed). IMHO, it's not worth the cycles.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
On Thu, Jun 25, 2026 at 02:05:10PM -0700, Bharath Rupireddy wrote:
+1 to what's said above. The merge burden is real (recently it took me
more than 30 minutes just to create patches for back-branches even
though most of the code remains the same in the function being
changed). IMHO, it's not worth the cycles.
The unnecessary backpatching noise it the most annoying part.
ereport() code may use multiple levels of parenthesis, and that tends
to make the handling of the conflicts across branches quite annoying.
Let's keep this code as it is.
--
Michael