pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)

Started by Noname3 months ago7 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t248640
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 20, 2026 at 07:13 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t248640_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t248640_1 && git checkout t248640_1

Patchset v1 (message #1) is on t248640_1

Jump to latest
#1Noname
m.litsarev@postgrespro.ru

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:

t248640_1
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
#2Sami Imseih
samimseih@gmail.com
In reply to: Noname (#1)
Re: pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)

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)

#3Noname
m.litsarev@postgrespro.ru
In reply to: Sami Imseih (#2)
Re: pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)

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.

#4Sami Imseih
samimseih@gmail.com
In reply to: Noname (#3)
Re: pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Sami Imseih (#2)
Re: pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)

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

#6Bharath Rupireddy
bharath.rupireddyforpostgres@gmail.com
In reply to: Tom Lane (#5)
Re: pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)

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

#7Michael Paquier
michael@paquier.xyz
In reply to: Bharath Rupireddy (#6)
Re: pg_stat_statements: Remove (errcode...) framing parentheses in erport(...)

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