Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches
Date
Msg-id 20160401195948.GA91873@alvherre.pgsql
Whole thread Raw
In response to Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches  (Craig Ringer <craig@2ndquadrant.com>)
List pgsql-hackers
Craig Ringer wrote:

> Note that I can't use PG_GETARG_TRANSACTIONID directly since it's a macro
> defined only in xid.c . It didn't seem worth extracting it and moving it to
> postgres.h (where the other non-ADT-specific PG_GETARG_ macros are) or its
> own new header just for this, so I've spelled it out each time.

Hm, we should probably fix this sometime.  We used to think of Xids are
purely internal objects, but they are becoming more and more visible to
userland.

> I now remember that that's part of why I used bigint as an argument type.
> The other part is that txid_current() returns bigint and there's no cast
> from bigint to xid. So the tests would have to CREATE CAST or cast via
> 'text'. They now do the latter.

I was rather surprised at this -- I thought that it'd break when the Xid
epoch got further than 0 (because casting from a number larger than 2^32
would throw an overflow error, I thought), but as it turns out the cast
from text to XID automatically discards the higher-order bits when a
higher epoch is used.  I imagine this is intentional, but it's not
actually documented in xidin() at all.  Also, I'm not sure what happens
if strtoul() refuses to work on > INT_MAX values ... it's documented to
set errno to ERANGE, so I assume the whole chain simply fails.  Maybe
there's just no platform on which that happens anymore.


With regards to the rest of the patch, I decided not to use your
approach: it seemed a bit odd to accept NULL values there.  I changed
the query to use COALESCE in the value that returned null instead.  With
your change, the function takes a null and interprets it as InvalidXid
anyway, so it seems to work fine.  (I also tested it manually.)

Let's see what hamster has to say about this.

It would be really good to have other buildfarm members run this code.
Currently only Hamster does, and only because Michaël patched the
buildfarm script ...

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



pgsql-hackers by date:

Previous
From: Robbie Harwood
Date:
Subject: Re: [PATCH v10] GSSAPI encryption support
Next
From: Alvaro Herrera
Date:
Subject: Re: pgbench - remove unused clientDone parameter