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

On Tue, Feb 4, 2014 at 6:15 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>
>> + /*----------------------------------------------------------
>> + * Relational operators for LSNs
>> + *---------------------------------------------------------*/
>
> Isn't it just operators? They aren't really relational...
Operators for LSNs?

>> *** 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.
So let's go with DatumGetLSN and LSNGetDatum instead...

>> *** 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.
Funnily, the patch attached in my previous mail did not include all
the diffs, it is an error with filterdiff that I use to generate
context diff patches... My original branch includes the following
diffs as well in slotfuncs.c for the second patch:
diff --git a/src/backend/replication/slotfuncs.c
b/src/backend/replication/slotfuncs.c
index 98a860e..68ecdcd 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -141,8 +141,6 @@ 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);
@@ -164,9 +162,6 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)

memset(nulls, 0, sizeof(nulls));

- snprintf(restart_lsn_s, sizeof(restart_lsn_s), "%X/%X",
- (uint32) (restart_lsn >> 32),
(uint32) restart_lsn);
-
i = 0;
values[i++] = CStringGetTextDatum(slot_name);
if (database == InvalidOid)
@@ -180,7 +175,7 @@ pg_get_replication_slots(PG_FUNCTION_ARGS)
else
nulls[i++] = true;
if (restart_lsn != InvalidTransactionId)
- values[i++] = CStringGetTextDatum(restart_lsn_s);
+ values[i++] = restart_lsn;
else
nulls[i++] = true;

Anything else?
--
Michael