On Tue, Sep 21, 2021 at 2:53 PM Masahiko Sawada <sawada.mshk@gmail.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
