On Wed, Jul 15, 2026 at 05:23:05PM +0530, Amit Kapila wrote: > On Wed, Jul 15, 2026 at 8:28 AM Noah Misch <[email protected]> wrote: > > On Mon, Jul 13, 2026 at 03:37:54PM +0530, Amit Kapila wrote: > > > On Fri, Jul 10, 2026 at 10:22 AM Noah Misch <[email protected]> wrote: > > > > Fable 5 also wrote a lot more that neither it nor I confirmed by test > > > > case > > > > construction. I'm attaching the report; feel free to disregard. > > > > Finding-2 > > > > about default_transaction_read_only=on looks worth fixing if true, > > > > > > Agreed on Finding-2 as well. The issue is that the sequencesync > > > worker sets the value via SetSequence(), which calls > > > PreventCommandIfReadOnly("setval()") for non-temp sequences, so with > > > "default_transaction_read_only=on" on the subscriber the worker's > > > transaction is read-only and sequence sync fails and never reaches > > > READY. Table apply is unaffected only because > > > ExecSimpleRelationInsert() bypasses the executor's > > > ExecCheckXactReadOnly() path which is an undocumented, untested detail > > > rather than a stated guarantee. > > > > > > For a minimal backpatch, we can force the sequencesync worker to run > > > read-write (e.g. set default_transaction_read_only=off for its session > > > at startup) so it matches table apply, plus a test that sets the GUC > > > on the subscriber and verifies sequences reach READY. Separately, it's > > > worth documenting that logical replication apply is exempt from > > > default_transaction_read_only — it's a per-transaction default meant > > > to guard user writes and never makes the node physically read-only — > > > and making that exemption explicit for all logical replication workers > > > so tables no longer rely on the bypass. What do you think? > > > > I wouldn't document those things. default_transaction_read_only just has > > the > > user write "BEGIN READ WRITE" instead of plain "BEGIN". Hence, it's more > > like > > an "are you sure?" prompt than a restrictive guard. It's no surprise that > > logical replication apply achieves the equivalent of BEGIN READ WRITE; I > > don't > > see that outcome as an exemption. > > > > If easy, I would have the worker do the C equivalent of "BEGIN READ WRITE" > > instead of actually changing the GUC. That makes it clear exactly which > > areas > > are overriding the default. But changing the GUC is fine. > > Fair enough. I think this means we need to set XactReadOnly as false > each time after StartTransactionCommand() (where required) as we are > doing in snapbuild.c. There seems to be multiple places and some care > is required unless we want to do it each time after > StartTransactionCommand(). So, I prefer the GUC approach
In that case, changing the GUC works for me.
