On Thu, Oct 30, 2025 at 12:56 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Oct 29, 2025 at 5:10 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> >
> > 8.
> > @@ -136,6 +137,12 @@ create_logical_replication_slot(char *name, char *plugin,
> > temporary ? RS_TEMPORARY : RS_EPHEMERAL, two_phase,
> > failover, false);
> >
> > + /*
> > + * Ensure the logical decoding is enabled before initializing the logical
> > + * decoding context.
> > + */
> > + EnsureLogicalDecodingEnabled();
> > …
> >
> > EnsureLogicalDecodingEnabled(void)
> > {
> > Assert(MyReplicationSlot);
> >
> > if (wal_level != WAL_LEVEL_REPLICA)
> > return;
> >
> > /* Prepare and start the activation process if it's disabled */
> > if (!start_logical_decoding_status_change(true))
> > return;
> >
> > I see that EnsureLogicalDecodingEnabled() sometimes returns without
> > enabling decoding (like when wal_level is not a replica). It is
> > possible that the same is not possible in this code flow, but won't it
> > be better to get the return value and assert if this function returns
> > false?
>
> Do you mean to check the value returned by
> start_logical_decoding_change()? I could not understand what assertion
> we can put there.
>
I mean to check the return value of EnsureLogicalDecodingEnabled()
because there are two code paths as quoted in my message from where if
this function returned, the logical decoding won't be enabled.
> Related to this point, I thought it might be better to raise an error
> if this function is called when wal_level='minimal' instead of doing
> nothing because it cannot ensure logical decoding enabled by calling
> this function.
>
Yeah, that will also work for one of the code paths in
EnsureLogicalDecodingEnabled() but I think we can still return without
enabling the decoding from the other code path quoted in my message.
--
With Regards,
Amit Kapila.
