Re: Binary support for pgoutput plugin

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Dave Cramer <davecramer(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(at)paquier(dot)xyz>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary support for pgoutput plugin
Date: 2020-07-09 08:48:06
Message-ID: 95E2599C-8898-4F0A-999F-8F55CBFECC2D@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 7 Jul 2020, at 22:53, Dave Cramer <davecramer(at)gmail(dot)com> wrote:

> I have put all your requests other than the indentation as that can be dealt with by pg_indent into another patch which I 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 setting up
> 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2020-07-09 09:11:35 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Previous Message kato-sho@fujitsu.com 2020-07-09 08:43:06 RE: Performing partition pruning using row value