Binary support for pgoutput plugin

On Mon, Jun 03, 2019 at 10:49:54AM -0400, Dave Cramer wrote:
> Is there a reason why pgoutput sends data in text format? Seems to
> me that sending data in binary would provide a considerable
> performance improvement.

Are you seeing something that suggests that the text output is taking
a lot of time or other resources?

Hi,

On 2019-06-04 15:47:04 -0400, Dave Cramer wrote:
> On Mon, 3 Jun 2019 at 20:54, David Fetter <[hidden email]> wrote:
>
> > On Mon, Jun 03, 2019 at 10:49:54AM -0400, Dave Cramer wrote:
> > > Is there a reason why pgoutput sends data in text format? Seems to
> > > me that sending data in binary would provide a considerable
> > > performance improvement.
> >
> > Are you seeing something that suggests that the text output is taking
> > a lot of time or other resources?
> >
> > Actually it's on the other end that there is improvement. Parsing text
> takes much longer for almost everything except ironically text.

It's on both sides, I'd say. E.g. float (until v12), timestamp, bytea
are all much more expensive to convert from binary to text.


> To be more transparent there is some desire to use pgoutput for something
> other than logical replication. Change Data Capture clients such as
> Debezium have a requirement for a stable plugin which is shipped with core
> as this is always available in cloud providers offerings. There's no reason
> that I am aware of that they cannot use pgoutput for this.

Except that that's not pgoutput's purpose, and we shouldn't make it
meaningfully more complicated or slower to achieve this. Don't think
there's a conflict in this case though.

> There's also no reason that I am aware that binary outputs can't be
> supported.

Well, it *does* increase version dependencies, and does make replication
more complicated, because type oids etc cannot be relied to be the same
on source and target side.

> The protocol would have to change slightly and I am working
> on a POC patch.

Hm, what would have to be changed protocol wise? IIRC that'd just be a
different datum type? Or is that what you mean?
                pq_sendbyte(out, 't');  /* 'text' data follows */

IIRC there was code for the binary protocol in a predecessor of
pgoutput.

I think if we were to add binary output - and I think we should - we
ought to only accept a patch if it's also used in core.

Hi,

On 2019-06-04 16:39:32 -0400, Dave Cramer wrote:
> On Tue, 4 Jun 2019 at 16:30, Andres Freund <[hidden email]>
> wrote:
> > > There's also no reason that I am aware that binary outputs can't be
> > > supported.
> >
> > Well, it *does* increase version dependencies, and does make replication
> > more complicated, because type oids etc cannot be relied to be the same
> > on source and target side.
> >
> I was about to agree with this but if the type oids change from source
> to target you still can't decode the text version properly. Unless I
> mis-understand something here ?

The text format doesn't care about oids. I don't see how it'd be a
problem?  Note that some people *intentionally* use different types from
source to target system when logically replicating. So you can't rely on
the target table's types under any circumstance.

I think you really have to use the textual type which we already write
out (cf logicalrep_write_typ()) to call the binary input functions. And
you can send only data as binary that's from builtin types - otherwise
there's no guarantee at all that the target system has something
compatible. And even if you just assumed that all extensions etc are
present, you can't transport arrays / composite types in binary: For
hard to discern reasons we a) embed type oids in them b) verify them. b)
won't ever work for non-builtin types, because oids are assigned
dynamically.

> > I think if we were to add binary output - and I think we should - we
> > ought to only accept a patch if it's also used in core.
> >
>
> Certainly; as not doing so would make my work completely irrelevant for my
> purpose.

What I mean is that the builtin logical replication would have to use
this on the receiving side too.

Hi,

On 2019-06-05 00:05:02 +0200, David Fetter wrote:
> Would it make sense to work toward a binary format that's not
> architecture-specific? I recall from COPY that our binary format is
> not standardized across, for example, big- and little-endian machines.

I think you recall wrongly. It's obviously possible that we have bugs
around this, but output/input routines are supposed to handle a
endianess independent format. That usually means that you have to do
endianess conversions, but that doesn't make it non-standardized.

Hi,

On 05/06/2019 00:08, Andres Freund wrote:
> Hi,
>
> On 2019-06-05 00:05:02 +0200, David Fetter wrote:
>> Would it make sense to work toward a binary format that's not
>> architecture-specific? I recall from COPY that our binary format is
>> not standardized across, for example, big- and little-endian machines.
>
> I think you recall wrongly. It's obviously possible that we have bugs
> around this, but output/input routines are supposed to handle a
> endianess independent format. That usually means that you have to do
> endianess conversions, but that doesn't make it non-standardized.
>

Yeah, there are really 3 formats of data we have, text protocol, binary
network protocol and internal on disk format. The internal on disk
format will not work across big/little-endian but network binary
protocol will.

FWIW I don't think the code for binary format was included in original
logical replication patch (I really tried to keep it as minimal as
possible), but the code and protocol is pretty much ready for adding that.

That said, pglogical has code which handles this (I guess Andres means
that by predecessor of pgoutput) so if you look for example at the
write_tuple/read_tuple/decide_datum_transfer in
https://github.com/2ndQuadrant/pglogical/blob/REL2_x_STABLE/pglogical_proto_native.c
that can help you give some ideas on how to approach this.

Hi,

On Wed, 5 Jun 2019 at 07:18, Petr Jelinek <[hidden email]> wrote:

Hi,

On 05/06/2019 00:08, Andres Freund wrote:
> Hi,
>
> On 2019-06-05 00:05:02 +0200, David Fetter wrote:
>> Would it make sense to work toward a binary format that's not
>> architecture-specific? I recall from COPY that our binary format is
>> not standardized across, for example, big- and little-endian machines.
>
> I think you recall wrongly. It's obviously possible that we have bugs
> around this, but output/input routines are supposed to handle a
> endianess independent format. That usually means that you have to do
> endianess conversions, but that doesn't make it non-standardized.
>

Yeah, there are really 3 formats of data we have, text protocol, binary
network protocol and internal on disk format. The internal on disk
format will not work across big/little-endian but network binary
protocol will.

FWIW I don't think the code for binary format was included in original
logical replication patch (I really tried to keep it as minimal as
possible), but the code and protocol is pretty much ready for adding that.

Yes, I looked through the public history and could not find it. Thanks for confirming. 

That said, pglogical has code which handles this (I guess Andres means
that by predecessor of pgoutput) so if you look for example at the
write_tuple/read_tuple/decide_datum_transfer in
https://github.com/2ndQuadrant/pglogical/blob/REL2_x_STABLE/pglogical_proto_native.c
that can help you give some ideas on how to approach this.

Thanks for the tip!

Hi

On June 5, 2019 8:51:10 AM PDT, Dave Cramer <[hidden email]> wrote:
>On Wed, 5 Jun 2019 at 07:21, Dave Cramer <[hidden email]> wrote:
>
>> Hi,
>>
>>
>> On Wed, 5 Jun 2019 at 07:18, Petr Jelinek
><[hidden email]>
>> wrote:
>>
>>> Hi,
>>>
>>> On 05/06/2019 00:08, Andres Freund wrote:
>>> > Hi,
>>> >
>>> > On 2019-06-05 00:05:02 +0200, David Fetter wrote:
>>> >> Would it make sense to work toward a binary format that's not
>>> >> architecture-specific? I recall from COPY that our binary format
>is
>>> >> not standardized across, for example, big- and little-endian
>machines.
>>> >
>>> > I think you recall wrongly. It's obviously possible that we have
>bugs
>>> > around this, but output/input routines are supposed to handle a
>>> > endianess independent format. That usually means that you have to
>do
>>> > endianess conversions, but that doesn't make it non-standardized.
>>> >
>>>
>>> Yeah, there are really 3 formats of data we have, text protocol,
>binary
>>> network protocol and internal on disk format. The internal on disk
>>> format will not work across big/little-endian but network binary
>>> protocol will.
>>>
>>> FWIW I don't think the code for binary format was included in
>original
>>> logical replication patch (I really tried to keep it as minimal as
>>> possible), but the code and protocol is pretty much ready for adding
>that.
>>>
>> Yes, I looked through the public history and could not find it.
>Thanks for
>> confirming.
>>
>>>
>>> That said, pglogical has code which handles this (I guess Andres
>means
>>> that by predecessor of pgoutput) so if you look for example at the
>>> write_tuple/read_tuple/decide_datum_transfer in
>>>
>>>
>https://github.com/2ndQuadrant/pglogical/blob/REL2_x_STABLE/pglogical_proto_native.c
>>> that can help you give some ideas on how to approach this.
>>>
>>
>> Thanks for the tip!
>>
>
>Looking at:
>https://github.com/postgres/postgres/blob/8255c7a5eeba8f1a38b7a431c04909bde4f5e67d/src/backend/replication/pgoutput/pgoutput.c#L163
>
>this seems completely ignored. What was the intent?

That's about the output of the plugin, not the datatypes. And independent of text/binary output, the protocol contains non-printable chars.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Hi,

On 2019-06-05 18:47:57 -0400, Dave Cramer wrote:
> So one of the things they would like added is to get not null information
> in the schema record. This is so they can mark the field Optional in Java.
> I presume this would also have some uses in other languages. As I
> understand it this would require a protocol bump. If this were to be
> accepted are there any outstanding asks that would useful to add if we were
> going to bump the protocol?

I'm pretty strongly opposed to this. What's the limiting factor when
adding such information? I think clients that want something like this
ought to query the database for catalog information when getting schema
information.