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@sss.pgh.pa.us 
> <mailto:tgl@sss.pgh.pa.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/