Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Craig Ringer <craig(at)2ndquadrant(dot)com>
Cc: Petr Jelinek <petr(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches
Date: 2016-04-01 19:59:48
Message-ID: 20160401195948.GA91873@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers 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

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Alvaro Herrera 2016-04-01 20:12:33 pgsql: pgbench: Remove unused parameter
Previous Message Tom Lane 2016-04-01 19:48:35 pgsql: Omit null rows when applying the Haas-Stokes estimator for ndist

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2016-04-01 20:13:18 Re: pgbench - remove unused clientDone parameter
Previous Message Jason Petersen 2016-04-01 19:26:26 Re: Getting Citus into (Debian) PGDG