Thread: 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.