Hi, On 2014-02-04 10:23:14 +0900, Michael Paquier wrote: > On Tue, Feb 4, 2014 at 10:10 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Michael Paquier <michael.paqu...@gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
