On Mon, Sep 27, 2021 at 11:40 PM Euler Taveira <eu...@eulerto.com> wrote:
>
> On Mon, Sep 27, 2021, at 4:09 AM, tushar wrote:
>
> so are we silently ignoring this parameter as it is not supported on v14 ?
>
> Yes. Because two_phase is a supported parameter for v15 (your current
> subscriber). The issue is that this parameter are not forwarded to publisher
> because its version (v14) does not support it. Since we do not have a
> connection before parse_subscription_options(), publisher server version is
> unknown. Hence, it does not know if that specific parameter is supported on
> publisher. I'm not sure it is worth parsing the options again after a
> replication connection is available just to check those parameters that don't
> work on all supported server versions.
>
> IMO we can provide messages during the connection (see
> libpqrcv_startstreaming()) instead of while executing CREATE/ALTER
> SUBSCRIPTION. Something like:
>
> if (options->proto.logical.twophase &&
> PQserverVersion(conn->streamConn) >= 150000)
> appendStringInfoString(&cmd, ", two_phase 'on'");
> else if (options->proto.logical.twophase)
> ereport(DEBUG1,
> (errmsg_internal("parameter \"two_phase\" is not
> supported on the publisher")));
>
> It is a DEBUG message because it can be annoying when the subscriber cannot
> connect to the publisher.
>
> The output plugin also raises an error if the subscriber sends the two_phase
> parameter. See pgoutput_startup(). The subscriber could probably send all
> parameters and the output plugin would be responsible to report an error. I
> think the author decided to not do it because it is not an user-friendly
> approach.
>
True, and the same behavior was already there for 'binary' and
'streaming' options. Shall we document this instead of DEBUG message
or probably along with DEBUG message?
--
With Regards,
Amit Kapila.
