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

On 2014-02-04 19:17:51 +0900, Michael Paquier wrote:
> 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?

Fine with me.

> >> + #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...

Sup.

> >> *** 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

Ah, then it makes more sense.

> 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;

Isn't that missing a LSNGetDatum()? Also, isn't it lacking the
corresponding pg_proc change?

Greetings,

Andres Freund

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