Re: should we add a XLogRecPtr/LSN SQL type?

Hi,

On 2014-02-04 10:23:14 +0900, Michael Paquier wrote:
> On Tue, Feb 4, 2014 at 10:10 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> >> Please find attached a patch implementing lsn as a datatype, based on
> >> the one Robert wrote a couple of years ago.
> >
> >> Patch contains regression tests as well as a bit of documentation.
> >> Perhaps this is too late for 9.4, so if there are no objections I'll
> >> simply add this patch to the next commit fest in June for 9.5.
> >
> > I may have lost count, but aren't a bunch of the affected functions new
> > in 9.4? If so, there's a good argument to be made that we should get
> > this in now, rather than waiting and having an API change for those
> > functions in 9.5.

Yes, that sounds sensible.

> + /*----------------------------------------------------------
> + * Relational operators for LSNs
> + *---------------------------------------------------------*/

Isn't it just operators? They aren't really relational...

> *** 302,307 **** extern struct varlena *pg_detoast_datum_packed(struct varlena * datum);
> --- 303,309 ----
> #define PG_RETURN_CHAR(x) return CharGetDatum(x)
> #define PG_RETURN_BOOL(x) return BoolGetDatum(x)
> #define PG_RETURN_OID(x) return ObjectIdGetDatum(x)
> + #define PG_RETURN_LSN(x) return LogSeqNumGetDatum(x)
> #define PG_RETURN_POINTER(x) return PointerGetDatum(x)
> #define PG_RETURN_CSTRING(x) return CStringGetDatum(x)
> #define PG_RETURN_NAME(x) return NameGetDatum(x)
> *** a/src/include/postgres.h
> --- b/src/include/postgres.h
> ***************
> *** 484,489 **** typedef Datum *DatumPtr;
> --- 484,503 ----
> #define ObjectIdGetDatum(X) ((Datum) SET_4_BYTES(X))
>
> /*
> + * DatumGetLogSeqNum
> + * Returns log sequence number of a datum.
> + */
> +
> + #define DatumGetLogSeqNum(X) ((XLogRecPtr) GET_8_BYTES(X))

I am not a fan of LogSegNum. I think at this point fewer people
understand that than LSN. There's also no reason to invent a third term
for LSNs. We'd have LSN, XLogRecPtr, and LogSeqNum.

> *** a/src/backend/replication/slotfuncs.c
> --- b/src/backend/replication/slotfuncs.c
> ***************
> *** 141,148 **** pg_get_replication_slots(PG_FUNCTION_ARGS)
> bool active;
> Oid database;
> const char *slot_name;
> -
> - char restart_lsn_s[MAXFNAMELEN];
> int i;
>
> SpinLockAcquire(&slot->mutex);
> --- 141,146 ----

Unrelated change.

Looks reasonable on a first look. Thanks!

Greetings,

Andres Freund

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