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

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

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