On Mon, Jul 27, 2026 at 11:56:46AM -0300, Euler Taveira wrote:
> If you use PG_ENSURE_ERROR_CLEANUP with nested blocks, the compiler warns about
> shadow locals. There is no harm since each PG_TRY and PG_CATCH uses its own
> context stack. The current code doesn't have an example of such usage but
> pglogical [1] does. I don't know if other extensions have the same issue too.
>
> The PG_TRY and PG_CATCH already accept an optional parameter (commit
> 112f0225dbfe) to avoid this exact issue. Since PG_ENSURE_ERROR_CLEANUP /
> PG_END_ENSURE_ERROR_CLEANUP already use PG_TRY / PG_CATCH, add an optional
> parameter too. (I opted to a variadic argument argument -- mimicking how PG_TRY
> already handles its optional suffix parameter -- because it avoids changing existing callers).
This line of arguments makes sense here.
If I check out the pglogical code at the top of its REL2_x_STABLE
branch and try to compile it with a Postgres at the top of
REL_19_STABLE and your patch applied, then I still check shadow
variable warnings:
In file included from /home/popo/pgsql/include/server/postgres.h:49,
from pglogical_sync.c:14:
pglogical_sync.c: In function ‘pglogical_sync_subscription’:
/home/popo/pgsql/include/server/utils/elog.h:390:29: warning:
declaration of ‘_save_exception_stack’ shadows a previous local
[-Wshadow=compatible-local]
390 | sigjmp_buf *_save_exception_stack##__VA_ARGS__
= PG_exception_stack; \
| ^~~~~~~~~~~~~~~~~~~~~
Something I am missing or a change on the pglogical side I am missing
perhaps?
--
Michael
