On Thu, Mar 30, 2023 at 11:11 AM Jelte Fennema <postgres@jeltef.nl> wrote:
> Regarding the actual patch. I think the code looks good. Mainly the
> tests and docs are lacking for the new option. Like I said for the
> tests you can borrow the tests I updated for my v2 patch, I think
> those should work fine for the new option.
I took a look at that but I didn't really feel like that was quite the
direction I wanted to go. I'd actually like to separate the tests of
the new option out into their own file, so that if for some reason we
decide we want to remove it in the future, it's easier to nuke all the
associated tests. Also, quite frankly, I think we've gone way
overboard in terms of loading too many tests into a single file, with
the result that it's very hard to understand exactly what and all the
file is actually testing and what it's intended to be testing. So the
attached 0002 does it that way. I've also amended 0001 and 0002 with
documentation changes that I hope are appropriate.
I noticed along the way that my earlier commits had missed one place
that needed to be updated by the pg_create_subscription patch I
created earlier. A fix for that is included in 0001, but it can be
broken out and committed separately if somebody feels strongly about
it. I personally don't think it's worth it.
--
Robert Haas
EDB: http://www.enterprisedb.com
