Re: Proposal: Conflict log history table for Logical Replication - Mailing list pgsql-hackers

On Wed, 6 May 2026 at 19:35, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, May 6, 2026 at 6:28 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Wed, May 6, 2026 at 4:31 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Wed, May 6, 2026 at 3:01 PM shveta malik <shveta.malik@gmail.com> wrote:
> > > >
> > > > On Wed, May 6, 2026 at 9:24 AM vignesh C <vignesh21@gmail.com> wrote:
> > > > >
> > > > > Few comments:
> > > > > 1) Currently we allow renaming of pg_conflict schema, this might be ok
> > > > > as we allow other sysem schema like pg_catalog and pg_toast also.
> > > > > postgres=# alter schema pg_conflict rename to test_conflict;
> > > > > ALTER SCHEMA
> > > > >
> > > >
> > > > I agree that we allow renaming other schemas including pg_toast, but I
> > > > am not sure if this is consciously made decision, see BUG #18281 ast
> > > > [1]. I don't favour allowing renaming pg_conflict for 2 reasons:
> > > >
> > > > 1) Because Postgres explicitly blocks renaming schemas to a name
> > > > starting with 'pg_'. If an admin accidentally renames 'pg_conflict' to
> > > > something else, they are permanently locked out from renaming it back.
> > > >
> > > > 2) While the core worker might survive a rename via OID lookups;
> > > > external scripts, extensions, and monitoring tools will likely
> > > > hardcode the 'pg_conflict' string. If the schema is renamed, these
> > > > tools will fail.
> > > >
> > >
> > > I think we shouldn't go out of our way to disallow superusers to
> > > rename pg_conflict schema similar to other cases. We can try to
> > > prevent hard-coding schema names where possible but not sure we can
> > > guarantee that nothing related to pg_conflict schema won't break as
> > > shown by you in the following similar case for pg_conflict.
> > >
> > > > One such example  of scripts breaking is present event in Postgres. I
> > > > did the following, and most of psql commands started failing after
> > > > that due to hard-coded pg_catalog name in them.
> > > >
> > > > postgres=# alter schema pg_catalog rename to catalog_new;
> > > > ALTER SCHEMA
> > > >
> > > > postgres=# \d catalog_new.*
> > > > ERROR:  relation "pg_catalog.pg_class" does not exist
> > > > LINE 5: FROM pg_catalog.pg_class c
> > > >
> > > > [1]: https://www.postgresql.org/message-id/flat/18281-5b1b6c5991d345aa%40postgresql.org
> >
> > I can see pg_toast and pg_catalog schema also hard coded in couple of
> > places e.g.
> >
> > listPartitionedTables()
> > {
> > if (!pattern)
> > appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
> > " AND n.nspname !~ '^pg_toast'\n"
> > " AND n.nspname <> 'information_schema'\n");
> > }
> >
> > I will analyze which all places we are hardcoding, I think on server
> > side code we can easily avoid but from client side e.g. describe we
> > might need to invent a way to identify the schema name, or we might
> > have to store it somewhere in pg_subscription etc, I don't think we
> > should go that route.
>
> Here is updated patch set

Thanks for the updated patches, the v30 version patch posted has few issues:
There is an assert at [1]:
TRAP: failed Assert("conflictlogrel != NULL"), File:
"../src/backend/replication/logical/conflict.c", Line: 195, PID: 59658
0xb3a472 <ExceptionalCondition+0x82> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x9433b8 <ReportApplyConflict+0x13c8> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x7d91fb <CheckAndReportConflict+0x2cb> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x7d8b4b <ExecSimpleRelationInsert+0x10b> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x96525b <apply_dispatch+0x23eb> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x966150 <start_apply+0x310> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x967010 <run_apply_worker+0x290> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x966d6d <ApplyWorkerMain+0x1d> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x90ff0c <BackgroundWorkerMain+0x1cc> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x914a25 <postmaster_child_launch+0x145> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x917b77 <maybe_start_bgworkers+0x1d7> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x9198f5 <ServerLoop+0x1c65> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x917156 <PostmasterMain+0x1116> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres
0x83c16d <main+0x48d> at
/tmp/cirrus-ci-build/build/tmp_install/usr/local/pgsql/bin/postgres

There are the following warnings at [2]:
[14:55:07.472] conflict.c:187:6: error: variable 'log_dest_clt' is
used uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
[14:55:07.472]   187 |         if (dest == CONFLICT_LOG_DEST_TABLE ||
dest == CONFLICT_LOG_DEST_ALL)
[14:55:07.472]       |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[14:55:07.472] conflict.c:193:6: note: uninitialized use occurs here
[14:55:07.472]   193 |         if (log_dest_clt)
[14:55:07.472]       |             ^~~~~~~~~~~~
[14:55:07.472] conflict.c:187:2: note: remove the 'if' if its
condition is always true
[14:55:07.472]   187 |         if (dest == CONFLICT_LOG_DEST_TABLE ||
dest == CONFLICT_LOG_DEST_ALL)
[14:55:07.472]       |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[14:55:07.472]   188 |                 log_dest_clt = true;
[14:55:07.472] conflict.c:176:21: note: initialize the variable
'log_dest_clt' to silence this warning
[14:55:07.472]   176 |         bool                    log_dest_clt;
[14:55:07.472]       |                                             ^
[14:55:07.472]       |                                              = false
[14:55:07.472] 1 error generated.

[1] -
https://api.cirrus-ci.com/v1/artifact/task/5630092254117888/testrun/build/testrun/subscription/026_stats/log/026_stats_subscriber.log
[2] - https://cirrus-ci.com/task/5770829742473216

Regards,
Vignesh