On Tue, Dec 19, 2023 at 12:07 PM Peter Smith <smithpb2...@gmail.com> wrote: > > On Tue, Dec 19, 2023 at 1:35 AM Emre Hasegeli <e...@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.