Re: Skipping logical replication transactions on subscriber side

On Tue, Sep 21, 2021 at 2:53 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> I've attached the updated version patches. Please review them.
>

A few review comments for the v14-0002 patch:

(1)
I suggest a small update to the patch comment:

BEFORE:
ALTER SUBSCRIPTION ... RESET command resets subscription
parameters. The parameters that can be set are streaming, binary,
synchronous_commit.

AFTER:
ALTER SUBSCRIPTION ... RESET command resets subscription
parameters to their default value. The parameters that can be reset
are streaming, binary, and synchronous_commit.

(2)
In the documentation, the RESETable parameters should be listed in the
same way and order as for SET:

BEFORE:
+ <para>
+ The parameters that can be reset are: <literal>streaming</literal>,
+ <literal>binary</literal>, <literal>synchronous_commit</literal>.
+ </para>
AFTER:
+ <para>
+ The parameters that can be reset are
<literal>synchronous_commit</literal>,
+ <literal>binary</literal>, and <literal>streaming</literal>.
+ </para>

Also, I'm thinking it would be beneficial to say the following before this:

RESET is used to set parameters back to their default value.

(3)
I notice that if you try to reset the slot_name, you get the following message:
postgres=# alter subscription sub reset (slot_name);
ERROR: unrecognized subscription parameter: "slot_name"

This is a bit misleading, because "slot_name" actually IS a
subscription parameter, just not resettable.
It would be better in this case if it said something like:
ERROR: not a resettable subscription parameter: "slot_name"

However, it seems that this is also an existing issue with SET (e.g.
for "refresh" or "two_phase"):
postgres=# alter subscription sub set (refresh=true);
ERROR: unrecognized subscription parameter: "refresh"

Regards,
Greg Nancarrow
Fujitsu Australia