Re: Proposal: Conflict log history table for Logical Replication

On Thu, Jun 4, 2026 at 12:28 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> > Here is the rebased version of the upgrade and \dRs patch which is
> > present in v45-0003 and v45-0004. There is no change in v45-0001 and
> > v45-0002, they are the same patch as in [1].
> > [1] - https://www.postgresql.org/message-id/CAFiTN-vWZR9%2BU-kg6d2L3J0WGr6fqESnjah8vrguVCmpEG7SMA%40mail.gmail.com
> >
>
> A few comments:
>
> 1)
> We have introduced errors like these:
>
> + errdetail("Conflict schema modifications are currently disallowed.")));
> + errmsg("cannot move objects into or out of the conflict schema")));
>
> But here:
>
> {
> /* Can't be system namespace */
> if (IsCatalogNamespace(schemaid) || IsToastNamespace(schemaid) ||
> IsConflictLogTableNamespace(schemaid))
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("cannot add schema \"%s\" to publication",
> get_namespace_name(schemaid)),
> errdetail("This operation is not supported for system schemas.")));
>
> We have included conflict-schema in system schemas, I feel here too,
> we shall make a new condition and error mentioning 'conflict schema'.
> Irrespective of how TOAST-scehma is handled here, we can have
> cosnsitent handling at-least for new conflcit-schema.

IMHO then the next immediate thought would be that for toast and
catalog the error is covering them under the system catalog and we are
diverging behaviour for the conflict schema. I think the current
behaviour is fine.

> 2)
> +#include "catalog/dependency.h"
> +#include "commands/subscriptioncmds.h"
>
> conflict.c cimpiles without above 2 inclusions.

Will fix

>
> v45-0002:
> 3)
> old name (pg_conflict_log_for_subid_16396) referred in commit message of 002

Will fix

> 4)
> Since ProcessPendingConflictLogTuple() is called in CATCH-block at
> multiple places, it would be better if
> ProcessPendingConflictLogTuple() had its own try-catch block. This
> block could log/WARN CLT-insertion errors, clear them, and allow the
> caller's original error to remain. In rare-scenarios, we may end up
> overriding the actual error with a CLT insertion error.
>

I think this make sense, let me put more thought on this.

--
Regards,
Dilip Kumar
Google