Re: Proposal: Conflict log history table for Logical Replication

On Thu, Oct 15, 2026 at 3:37 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> A few tirival comments for v81:
>
> v81-002:
> 2)
> drop_subscription.sgml:
>
> "If a conflict log table exists for the subscription (that is, when
> conflict_log_destination is set to table or all), DROP SUBSCRIPTION
> automatically drops the associated conflict log table. In that case,
> the subscription cannot be dropped while it has prepared transactions
> (see two_phase), because they may hold a lock on the conflict log
> table that is released only when the apply worker applies the
> corresponding COMMIT PREPARED or ROLLBACK PREPARED. Resolve these
> transactions first (see pg_prepared_xacts)."
>
> Above doc seems misleading. It gives an impression that CLT is dropped
> while sub can not be dropped in presence of prepared txns.
>
> Should we change it to:
> If conflict_log_destination is set to table or all, the subscription
> has an associated conflict log table, which DROP SUBSCRIPTION drops
> along with the subscription. However, DROP SUBSCRIPTION cannot be
> executed while the subscription has prepared transactions (see
> two_phase), because such a transaction may hold a lock on the conflict
> log table that is released only when the apply worker processes the
> corresponding COMMIT PREPARED or ROLLBACK PREPARED. Resolve these
> transactions first (see pg_prepared_xacts) before dropping the
> subscription.
>

okay agree, your suggestion looks good.

>
> 3)
> - "SELECT replica_identity::text FROM $clt_BA WHERE relname = 'tab_temporal';");
> -is($clt_row_temporal,
> - '{"id":"[1,2)","valid_at":"{[2020-01-01,2021-01-01)}"}',
> - 'replica identity uses table column type rather than index opckeytype'
> + "SELECT replica_identity::text FROM $clt_BA WHERE relname = 'tab_temporal';"
> );
> +is( $clt_row_temporal,
> + '{"id":"[1,2)","valid_at":"{[2020-01-01,2021-01-01)}"}',
> + 'replica identity uses table column type rather than index opckeytype');
>
> Why this change in 002, do we need it?
>

This formatting change is due to running pgperltidy on the file. It
should be removed from 002, and pgperltidy should be run on the file
in 001.

> 4)
> The test added in 0002 does not cover apply-worker's error in
> lock_conflictlog_relation(). Is that intentionally left?
>

No, it was just missed. We can add it.

--
Thanks,
Nisha