Re: should we add a XLogRecPtr/LSN SQL type? - Mailing list pgsql-hackers

On Tue, Feb 4, 2014 at 6:15 PM, Andres Freund <andres@2ndquadrant.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