RE: [HACKERS] logical decoding of two-phase transactions

Hi

On Tuesday, February 16, 2021 8:33 AM Peter Smith <smithpb2250(at)gmail(dot)com>
> On Fri, Feb 12, 2021 at 5:59 PM osumi(dot)takamichi(at)fujitsu(dot)com
> <osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
> > (2)
> >
> > File : v39-0006-Support-2PC-txn-Subscription-option.patch
> >
> > @@ -213,6 +219,15 @@ parse_subscription_options(List *options,
> > *streaming_given = true;
> > *streaming = defGetBoolean(defel);
> > }
> > + else if (strcmp(defel->defname, "two_phase") == 0 &&
> twophase)
> > + {
> > + if (*twophase_given)
> > + ereport(ERROR,
> > +
> (errcode(ERRCODE_SYNTAX_ERROR),
> > + errmsg("conflicting or
> redundant options")));
> > + *twophase_given = true;
> > + *twophase = defGetBoolean(defel);
> > + }
> >
> > You can add this test in subscription.sql easily with double twophase
> options.
>
> Thanks for the feedback. You are right.
>
> But in the pgoutput.c there are several other potential syntax errors
> "conflicting or redundant options" which are just like this "two_phase" one.
> e.g. there is the same error for options "proto_version", "publication_names",
> "binary", "streaming".
>
> AFAIK none of those other syntax errors had any regression tests. That is the
> reason why I did not include any new test for the "two_phase"
> option.
>
> So:
> a) should I add a new test per your feedback comment, or
> b) should I be consistent with the other similar errors, and not add the test?
>
> Of course it is easy to add a new test if you think option (a) is best.
>
> Thoughts?
OK. Then, we can think previously, such tests for other options are
regarded as needless because the result are too apparent.
Let's choose (b) to make the patch set aligned with other similar past codes.
Thanks.

Best Regards,
Takamichi Osumi