Re: Binary support for pgoutput plugin

Hi,

On 11/07/2020 14:14, Dave Cramer wrote:
>
>
> On Fri, 10 Jul 2020 at 14:21, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
> <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>> wrote:
>
> > 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.
>
> I took a quick look through the patch and had some concerns:
>
> * Please strip out the PG_VERSION_NUM and USE_INTEGER_DATETIMES checks.
> Those are quite dead so far as a patch for HEAD is concerned --- in
> fact,
> since USE_INTEGER_DATETIMES hasn't even been defined since v10 or so,
> the patch is actively doing the wrong thing there.  Not that it matters.
> This code will never appear in any branch where float timestamps could
> be a thing.
>
> * I doubt that the checks on USE_FLOAT4/8_BYVAL, sizeof(int),
> endiannness,
> etc, make any sense either.  Those surely do not affect the on-the-wire
> representation of values --- or if they do, we've blown it somewhere
> else.
> I'd just take out all those checks and assume that the binary
> representation is sufficiently portable.  (If it's not, it's more or
> less
> the user's problem, just as in binary COPY.)
>
>
> So is there any point in having them as options then ?
>

I am guessing this is copied from pglogical, right? We have them there
because it can optionally send data in the on-disk format (not the
network binary format) and there this matters, but for network binary
format they do not matter as Tom says.

--
Petr Jelinek
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/