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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to