On Tue, Dec 19, 2023 at 12:07 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Tue, Dec 19, 2023 at 1:35 AM Emre Hasegeli <emre@hasegeli.com> wrote:
> >
> > > Fair enough. I think we should push your first patch only in HEAD as
> > > this is a minor improvement over the current behaviour. What do you
> > > think?
> >
> > I agree.
>
> Patch 0001
>
> AFAICT parse_output_parameters possible errors are never tested. For
> example, there is no code coverage [1] touching any of these ereports.
>
> IMO there should be some simple test cases -- I am happy to create
> some tests if you agree they should exist.
>
I don't think having tests for all sorts of error checking will add
much value as compared to the overhead they bring.
> ~~~
>
> While looking at the function parse_output_parameters() I noticed that
> if an unrecognised option is passed the function emits an elog instead
> of an ereport
>
We don't expect unrecognized option here and for such a thing, we use
elog in the code. See the similar usage in
parseCreateReplSlotOptions().
I think we should move to 0002 patch now. In that, I suggest preparing
separate back branch patches.
--
With Regards,
Amit Kapila.