On Tue, Jul 21, 2026 at 9:47 AM shveta malik <[email protected]> wrote:
>
> On Mon, Jul 20, 2026 at 12:11 PM vignesh C <[email protected]> wrote:
> >
> > On Wed, 15 Jul 2026 at 17:23, Amit Kapila <[email protected]> wrote:
> > >
> > > So, I prefer the GUC approach and we are
> > > already overriding it for session_replication_role and search_path
> > > GUC's. The one minor difference could be that for PG19, we set it only
> > > for sequencesync worker and in HEAD for all workers.
> >
> > Here is a patch to handle the same. v4-0001 patch has the changes to
> > set default_transaction_read_only guc in InitializeLogRepWorker which
> > is common to logical replication workers in master branch and v4_PG19
> > patch has the changes to set default_transaction_read_only guc in
> > SequenceSyncWorkerMain which is only for sequence sync worker.
> >
>
> Few suggestions:
>
>
> 1)
> + /*
> + * Ignore default_transaction_read_only for logical replication workers,
> + * as they need to be able to modify subscriber-side state regardless of
> + * that setting.
> + */
>
> Shall we say:
> /*
>  * Ignore default_transaction_read_only for logical replication workers,
>  * as they need to modify subscriber-side state even when the GUC is enabled.
>  */
>

Either of these is fine but now that I have committed the patch, let's
keep the original one.

>
> 2)
>
> +##########
> +# Ensure that ALTER SUBSCRIPTION ... REFRESH SEQUENCES can still update
> +# sequence values and mark the sequence as ready even when
> +# default_transaction_read_only is enabled on the subscriber.
> +##########
>
>
> +$node_subscriber->safe_psql(
> + 'postgres', qq(
> + set default_transaction_read_only = off;
> + ALTER SUBSCRIPTION regress_seq_sub REFRESH SEQUENCES;
> +));
>
>
> In header we say that REFERSH SEQ can still work with
> default_transaction_read_only=on, but here we temprarily set
> default_transaction_read_only=off before 'REFRESH SEQUENCES'. This can
> be confusing for new readers.
>

The difference is between using SET and ALTER SYSTEM.

>
 We shall change the header comment and
> add one comment atop REFRESH.
>
> Suggestion:
>
> ##########
> # Ensure that the sequencesync worker triggered by
> # ALTER SUBSCRIPTION ... REFRESH SEQUENCES can still update sequence
> # values and mark the sequence as ready even when
> # default_transaction_read_only is enabled on the subscriber.
> ##########
>
> Atop REFRESH:
>
> # ALTER SUBSCRIPTION cannot be executed in a read-only transaction.
> # Disable default_transaction_read_only for this session before executing
> # REFRESH SEQUENCES. The sequencesync worker will still see
> # default_transaction_read_only as being enabled.
>

I am not sure if these additional comments are very helpful.

-- 
With Regards,
Amit Kapila.


Reply via email to