Обсуждение: Re: pgsql: Change timeline field of IDENTIFY_SYSTEM to int8

On 04.07.22 07:55, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> Change timeline field of IDENTIFY_SYSTEM to int8
> 
> Surely this patch is far from complete?
> 
> To start with, just a few lines down in IdentifySystem() the column
> is filled using Int32GetDatum not Int64GetDatum.  I will get some
> popcorn and await the opinions of the 32-bit buildfarm animals.
> 
> But what about whatever code is reading the output?  And what if
> that code isn't v16?  I can't believe that we can make a wire
> protocol change as summarily as this.

I think a client will either just read the string value and convert it 
to some numeric type without checking what type was actually sent, or if 
the client API is type-aware and automatically converts to a native type 
of some sort, then it will probably already support 64-bit ints.  Do you 
see some problem scenario?

I'm seeing a bigger problem now, which is that our client code doesn't 
parse bigger-than-int32 timeline IDs correctly.

libpqwalreceiver uses pg_strtoint32(), which will error on overflow.

pg_basebackup uses atoi(), so it will just truncate the value, except 
for READ_REPLICATION_SLOT, where it uses atol(), so it will do the wrong 
thing on Windows only.

There is clearly very little use for such near-overflow timeline IDs in 
practice.  But it still seems pretty inconsistent.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 04.07.22 07:55, Tom Lane wrote:
>> But what about whatever code is reading the output?  And what if
>> that code isn't v16?  I can't believe that we can make a wire
>> protocol change as summarily as this.

> I think a client will either just read the string value and convert it 
> to some numeric type without checking what type was actually sent, or if 
> the client API is type-aware and automatically converts to a native type 
> of some sort, then it will probably already support 64-bit ints.  Do you 
> see some problem scenario?

If the result of IDENTIFY_SYSTEM is always sent in text format, then
I agree that this isn't very problematic.  If there are any clients
that fetch it in binary mode, though, this is absolutely a wire
protocol break for them ... and no, I don't believe an unsupported
claim that they'd adapt automatically.

> I'm seeing a bigger problem now, which is that our client code doesn't 
> parse bigger-than-int32 timeline IDs correctly.

Yup.

            regards, tom lane

On 04.07.22 19:32, Tom Lane wrote:
> If the result of IDENTIFY_SYSTEM is always sent in text format, then
> I agree that this isn't very problematic.  If there are any clients
> that fetch it in binary mode, though, this is absolutely a wire
> protocol break for them

The result rows of the replication commands are always sent in text format.