Thread: pg_recvlogical fixes

Hi,

While testing wal2json, I faced some problems with pg_recvlogical.
Attached is a serie of patches that can improve pg_recvlogical. Patches
#2 and #3 are bugfixes (and should be applied to 9.5 too). Patch #1 is
not mandatory to 9.5.

Short description:

#1: add a bunch of checks to complain when using an option that is not
available in the specified action;
#2: there is a wrong check because startpos option can be specified with
--create-slot;
#3: doesn't ignore startpos in --create-slot because that action could
be specified together with --start action (that uses that option);


--
    Euler Taveira                   Timbira - http://www.timbira.com.br/
    PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

On 2015-10-21 11:52:31 -0300, Euler Taveira wrote:
> While testing wal2json, I faced some problems with pg_recvlogical. Attached
> is a serie of patches that can improve pg_recvlogical. Patches #2 and #3 are
> bugfixes (and should be applied to 9.5 too). Patch #1 is not mandatory to
> 9.5.

> #1: add a bunch of checks to complain when using an option that is not
> available in the specified action;

I'm not a fan of doing that. Doing that for every option tends to be
more annoying than helpful. E.g. pgbench's checks requires you to
pointlessly remove a lot of harmless options just to be able to pass -i.

> #2: there is a wrong check because startpos option can be specified with
> --create-slot;

> #3: doesn't ignore startpos in --create-slot because that action could be
> specified together with --start action (that uses that option);

It doesn't make sense to specify it with --create-slot though - you
can't even know what a the start position would mean before the slot is
created. You can't get older records than the ones at slot creation
time, and you can't know what a feature xlog position would mean.

Greetings,

Andres Freund

On 21-10-2015 18:36, Andres Freund wrote:
> On 2015-10-21 11:52:31 -0300, Euler Taveira wrote:
>> While testing wal2json, I faced some problems with pg_recvlogical. Attached
>> is a serie of patches that can improve pg_recvlogical. Patches #2 and #3 are
>> bugfixes (and should be applied to 9.5 too). Patch #1 is not mandatory to
>> 9.5.
>
>> #1: add a bunch of checks to complain when using an option that is not
>> available in the specified action;
>
> I'm not a fan of doing that. Doing that for every option tends to be
> more annoying than helpful. E.g. pgbench's checks requires you to
> pointlessly remove a lot of harmless options just to be able to pass -i.
>
Your comparison is not fair (8 x 28 options). I tend to agree that a lot 
of check is not nice to maintain. However, it is a good UI practice.

>> #2: there is a wrong check because startpos option can be specified with
>> --create-slot;
>
>> #3: doesn't ignore startpos in --create-slot because that action could be
>> specified together with --start action (that uses that option);
>
> It doesn't make sense to specify it with --create-slot though - you
> can't even know what a the start position would mean before the slot is
> created. You can't get older records than the ones at slot creation
> time, and you can't know what a feature xlog position would mean.
>
If it doesn't make sense, why --create-slot can be specified together 
with --start at all? Maybe a comment could clarify your point (circa 
line 902) because it is not clear in docs that a past LSN could not be 
specified or even a future LSN when both options are specified together.


--    Euler Taveira                   Timbira - http://www.timbira.com.br/   PostgreSQL: Consultoria, Desenvolvimento,
Suporte24x7 e Treinamento