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

From Craig Ringer
Subject Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches
Date
Msg-id CAMsr+YEu4ESrLOm2NzVxU-+ExxAMO3AqbexpGn7HozwRO5=3iw@mail.gmail.com
Whole thread Raw
In response to Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches  (Petr Jelinek <petr@2ndquadrant.com>)
Responses Re: Re: [COMMITTERS] pgsql: Enable logical slots to follow timeline switches
List pgsql-hackers
On 1 April 2016 at 11:13, Petr Jelinek <petr@2ndquadrant.com> wrote:
 

The function does following:
TransactionId new_xmin = (TransactionId) PG_GETARG_INT64(1);

This should be reasonable enough though; down-casting it will discard the high bits but that's fine when we know there's nothing interesting there.

TransactionId is a uint32 anyway, but I had a reason for the above. There's no cast from integer to xid, which is why I used bigint here, since we don't have a uint32 native SQL type. What I *should've* done is simply used quoted-literal arguments like

    XID '1234'

or cast via text

    1234::text::xid

so there was no need to use a bigint instead. I'll adjust appropriately so it uses PG_GETARG_TRANSACTIONID(1) and is declared as accepting 'xid'.
 
And we are passing NULL as that parameter, that could explain this.

If we're on an int64-by-value platform that'd be wrong but still work, it'd just be zero. On an int64-by-reference platform it could indeed segfault. So yes, I'd say that's the cause.
 
Also while reading it I wonder if the function should be defined with
xid type rather than bigint and use similar input code as xid.c.

Yes, it should.

I'll prep a follow-up patch. 

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Updated backup APIs for non-exclusive backups
Next
From: Michael Paquier
Date:
Subject: Re: OOM in libpq and infinite loop with getCopyStart()