From: Tom Lane <tgl@sss.pgh.pa.us>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: Mats Kindahl <mats.kindahl@gmail.com>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Use stack-allocated StringInfoData
Date: Thu, 06 Nov 2025 10:11:03 -0500
Message-ID: <1032332.1762441863@sss.pgh.pa.us> (raw)
In-Reply-To: <202511061027.6kun3grytx2d@alvherre.pgsql>
References: <202511061027.6kun3grytx2d@alvherre.pgsql>
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
> Yeah, I came across this a few weeks ago as well. I was thinking maybe
> we can expand the ereport() macro instead of duplicating things. It
> looks a bit ugly and I don't think we do it anywhere else ... I mean
> something like
> errstart(WARNING, NULL);
> if (cond)
> errmsg( ... );
> else
> errmsg( ... );
> // and so on
> errfinish(__FILE__, __LINE__, __func__);
> Is this too ugly to live? (I swear I had this on a branch already, but
> can't find it right this instant.)
I really dislike exposing errstart/errfinish to calling code like
that. I agree that check_publications_origin_tables is not pretty
as-is, but it's better than breaking open the ereport abstraction.
In the particular case at hand, it seems best to just write
two separate ereport calls for the check_table_sync and
not-check_table_sync cases, as David mentioned. Duplicating
the errdetail_plural call is slightly annoying but it's net less
code and less surprise than what's there now.
In general, in most other places where we have conditional message
text, it's done via ?: operators within the ereport macro,
for example this decades-old bit in sysv_shmem.c:
ereport(FATAL,
(errmsg("could not create shared memory segment: %m"),
errdetail("Failed system call was shmget(key=%lu, size=%zu, 0%o).",
(unsigned long) memKey, size,
IPC_CREAT | IPC_EXCL | IPCProtection),
(shmget_errno == EINVAL) ?
errhint("This error usually means that PostgreSQL's request for a shared memory "
"segment exceeded your kernel's SHMMAX parameter, or possibly that "
"it is less than "
"your kernel's SHMMIN parameter.\n"
"The PostgreSQL documentation contains more information about shared "
"memory configuration.") : 0,
(shmget_errno == ENOMEM) ?
errhint("This error usually means that PostgreSQL's request for a shared "
"memory segment exceeded your kernel's SHMALL parameter. You might need "
"to reconfigure the kernel with larger SHMALL.\n"
"The PostgreSQL documentation contains more information about shared "
"memory configuration.") : 0,
(shmget_errno == ENOSPC) ?
errhint("This error does *not* mean that you have run out of disk space. "
"It occurs either if all available shared memory IDs have been taken, "
"in which case you need to raise the SHMMNI parameter in your kernel, "
"or because the system's overall limit for shared memory has been "
"reached.\n"
"The PostgreSQL documentation contains more information about shared "
"memory configuration.") : 0));
We could do something similar here, but I'm not convinced
it's better than just writing two ereport's.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, alvherre@kurilemu.de, mats.kindahl@gmail.com, dgrowleyml@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Use stack-allocated StringInfoData
In-Reply-To: <1032332.1762441863@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox