> On 7 Jul 2020, at 22:53, Dave Cramer <davecramer@gmail.com> wrote:
> I have put all your requests other than the indentation as that can be dealt with by pg_indent into another patch
whichI reordered ahead of yours
> This should make it easier to see that all of your issues have been addressed.
Thanks for the update! Do note that my patch included a new file which is
missing from this patchset:
src/test/subscription/t/014_binary.pl
This is, IMO, the most interesting test of this feature so it would be good to
be included. It's a basic test and can no doubt be extended to be even more
relevant, but it's a start.
> I did not do the macro for updated, inserted, deleted, will give that a go tomorrow.
This might not be a blocker, but personally I think it would make the code more
readable. Anyone else have an opinion on this?
> I added these since this will now be used outside of logical replication and getting reasonable error messages when
settingup
> replication is useful. I added the \" and ,
I think the "lack of detail" in the existing error messages is intentional to
make translation easier, but I might be wrong here.
Reading through the new patch, and running the tests, I'm marking this as Ready
for Committer. It does need some cosmetic TLC, quite possibly just from
pg_indent but I didn't validate if it will take care of everything, and comment
touchups (there is still a TODO comment around wording that needs to be
resolved). However, I think it's in good enough shape for consideration at
this point.
cheers ./daniel