Further code review for pg_lsn data type.

Change input function error messages to be more consistent with what is
done elsewhere.  Remove a bunch of redundant type casts, so that the
compiler will warn us if we screw up.  Don't pass LSNs by value on
platforms where a Datum is only 32 bytes, per buildfarm.  Move macros
for packing and unpacking LSNs to pg_lsn.h so that we can include
access/xlogdefs.h, to avoid an unsatisfied dependency on XLogRecPtr.

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index 45f5131091cf9d602776865c1a80a74669cc0fc1..e2b528a24359beffbe5e5203af5cded71aed63e0 100644 (file)
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -38,17 +38,17 @@ pg_lsn_in(PG_FUNCTION_ARGS)
        if (len1 < 1 || len1 > MAXPG_LSNCOMPONENT || str[len1] != '/')
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-                                errmsg("invalid input syntax for transaction log location: \"%s\"", str)));
+                                errmsg("invalid input syntax for type pg_lsn: \"%s\"", str)));
        len2 = strspn(str + len1 + 1, "0123456789abcdefABCDEF");
        if (len2 < 1 || len2 > MAXPG_LSNCOMPONENT || str[len1 + 1 + len2] != '\0')
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-                                errmsg("invalid input syntax for transaction log location: \"%s\"", str)));
+                                errmsg("invalid input syntax for type pg_lsn: \"%s\"", str)));
 
        /* Decode result. */
        id = (uint32) strtoul(str, NULL, 16);
        off = (uint32) strtoul(str + len1 + 1, NULL, 16);
-       result = (XLogRecPtr) ((uint64) id << 32) | off;
+       result = ((uint64) id << 32) | off;
 
        PG_RETURN_LSN(result);
 }
@@ -56,7 +56,7 @@ pg_lsn_in(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_out(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr      lsn = (XLogRecPtr) PG_GETARG_LSN(0);
+       XLogRecPtr      lsn = PG_GETARG_LSN(0);
        char            buf[MAXPG_LSNLEN + 1];
        char       *result;
        uint32          id, off;
@@ -83,7 +83,7 @@ pg_lsn_recv(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_send(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn = (XLogRecPtr) PG_GETARG_LSN(0);
+       XLogRecPtr lsn = PG_GETARG_LSN(0);
        StringInfoData buf;
 
        pq_begintypsend(&buf);
@@ -99,8 +99,8 @@ pg_lsn_send(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_eq(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn1 = (XLogRecPtr) PG_GETARG_LSN(0);
-       XLogRecPtr lsn2 = (XLogRecPtr) PG_GETARG_LSN(1);
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
 
        PG_RETURN_BOOL(lsn1 == lsn2);
 }
@@ -108,8 +108,8 @@ pg_lsn_eq(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_ne(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn1 = (XLogRecPtr) PG_GETARG_LSN(0);
-       XLogRecPtr lsn2 = (XLogRecPtr) PG_GETARG_LSN(1);
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
 
        PG_RETURN_BOOL(lsn1 != lsn2);
 }
@@ -117,8 +117,8 @@ pg_lsn_ne(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_lt(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn1 = (XLogRecPtr) PG_GETARG_LSN(0);
-       XLogRecPtr lsn2 = (XLogRecPtr) PG_GETARG_LSN(1);
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
 
        PG_RETURN_BOOL(lsn1 < lsn2);
 }
@@ -126,8 +126,8 @@ pg_lsn_lt(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_gt(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn1 = (XLogRecPtr) PG_GETARG_LSN(0);
-       XLogRecPtr lsn2 = (XLogRecPtr) PG_GETARG_LSN(1);
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
 
        PG_RETURN_BOOL(lsn1 > lsn2);
 }
@@ -135,8 +135,8 @@ pg_lsn_gt(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_le(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn1 = (XLogRecPtr) PG_GETARG_LSN(0);
-       XLogRecPtr lsn2 = (XLogRecPtr) PG_GETARG_LSN(1);
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
 
        PG_RETURN_BOOL(lsn1 <= lsn2);
 }
@@ -144,8 +144,8 @@ pg_lsn_le(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_ge(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn1 = (XLogRecPtr) PG_GETARG_LSN(0);
-       XLogRecPtr lsn2 = (XLogRecPtr) PG_GETARG_LSN(1);
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
 
        PG_RETURN_BOOL(lsn1 >= lsn2);
 }
@@ -158,8 +158,8 @@ pg_lsn_ge(PG_FUNCTION_ARGS)
 Datum
 pg_lsn_mi(PG_FUNCTION_ARGS)
 {
-       XLogRecPtr lsn1 = (XLogRecPtr) PG_GETARG_LSN(0);
-       XLogRecPtr lsn2 = (XLogRecPtr) PG_GETARG_LSN(1);
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
        char            buf[256];
        Datum           result;
 

diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index db18a23bf6e1ce7f76cbf4c01835304901a8d7c2..8ea9ceb1b809602cfce516cbc2d13180857d7ec3 100644 (file)
--- a/src/include/catalog/pg_type.h
+++ b/src/include/catalog/pg_type.h
@@ -578,7 +578,7 @@ DESCR("UUID datatype");
 DATA(insert OID = 2951 ( _uuid                 PGNSP PGUID -1 f b A f t \054 0 2950 0 array_in array_out array_recv array_send - - array_typanalyze i x f 0 -1 0 0 _null_ _null_ _null_ ));
 
 /* pg_lsn */
-DATA(insert OID = 3220 ( pg_lsn                        PGNSP PGUID 8 t b U t t \054 0 0 3221 pg_lsn_in pg_lsn_out pg_lsn_recv pg_lsn_send - - - d p f 0 -1 0 0 _null_ _null_ _null_ ));
+DATA(insert OID = 3220 ( pg_lsn                        PGNSP PGUID 8 FLOAT8PASSBYVAL b U t t \054 0 0 3221 pg_lsn_in pg_lsn_out pg_lsn_recv pg_lsn_send - - - d p f 0 -1 0 0 _null_ _null_ _null_ ));
 DESCR("PostgreSQL LSN datatype");
 DATA(insert OID = 3221 ( _pg_lsn                       PGNSP PGUID -1 f b A f t \054 0 3220 0 array_in array_out array_recv array_send - - array_typanalyze d x f 0 -1 0 0 _null_ _null_ _null_ ));
 

diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index dd4c672bafcd58fa9fb1e6d265c48862d1a1d6b9..aed81cdc26b540555db83fa8b444c363038985eb 100644 (file)
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -230,7 +230,6 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena * datum);
 #define PG_GETARG_CHAR(n)       DatumGetChar(PG_GETARG_DATUM(n))
 #define PG_GETARG_BOOL(n)       DatumGetBool(PG_GETARG_DATUM(n))
 #define PG_GETARG_OID(n)        DatumGetObjectId(PG_GETARG_DATUM(n))
-#define PG_GETARG_LSN(n)        DatumGetLSN(PG_GETARG_DATUM(n))
 #define PG_GETARG_POINTER(n) DatumGetPointer(PG_GETARG_DATUM(n))
 #define PG_GETARG_CSTRING(n) DatumGetCString(PG_GETARG_DATUM(n))
 #define PG_GETARG_NAME(n)       DatumGetName(PG_GETARG_DATUM(n))
@@ -303,7 +302,6 @@ extern struct varlena *pg_detoast_datum_packed(struct varlena * datum);
 #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 LSNGetDatum(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)

diff --git a/src/include/postgres.h b/src/include/postgres.h
index c8b311fa22ef130c7d63778a4a6d9430d4355a14..a8a206d988bb0df4b23240ab878e3c916f660d38 100644 (file)
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -483,20 +483,6 @@ typedef Datum *DatumPtr;
 
 #define ObjectIdGetDatum(X) ((Datum) SET_4_BYTES(X))
 
-/*
- * DatumGetLSN
- *             Returns PostgreSQL log sequence number of a datum.
- */
-
-#define DatumGetLSN(X) ((XLogRecPtr) GET_8_BYTES(X))
-
-/*
- * LSNGetDatum
- *             Returns datum representation for a PostgreSQL log sequence number.
- */
-
-#define LSNGetDatum(X) ((Datum) SET_8_BYTES(X))
-
 /*
  * DatumGetTransactionId
  *             Returns transaction identifier value of a datum.

diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index dc2193006e586c615bad14278dea8a1ab09c77fa..981fcd6fa8c30bc584bca850025b6ccc40bbc561 100644 (file)
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -16,6 +16,7 @@
 #define PG_LSN_H
 
 #include "fmgr.h"
+#include "access/xlogdefs.h"
 
 extern Datum pg_lsn_in(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_out(PG_FUNCTION_ARGS);
@@ -31,4 +32,10 @@ extern Datum pg_lsn_ge(PG_FUNCTION_ARGS);
 
 extern Datum pg_lsn_mi(PG_FUNCTION_ARGS);
 
+#define DatumGetLSN(X) ((XLogRecPtr) DatumGetInt64(X))
+#define LSNGetDatum(X) (Int64GetDatum((int64) (X)))
+
+#define PG_GETARG_LSN(n)        DatumGetLSN(PG_GETARG_DATUM(n))
+#define PG_RETURN_LSN(x)        return LSNGetDatum(x)
+
 #endif   /* PG_LSN_H */

diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 8cfe28e4b8ac88801d00587597787588f71000cc..01d298397bc15b0a6d7c9b6ea3357695e2e60ee7 100644 (file)
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -7,23 +7,23 @@ INSERT INTO PG_LSN_TBL VALUES ('0/0');
 INSERT INTO PG_LSN_TBL VALUES ('FFFFFFFF/FFFFFFFF');
 -- Incorrect input
 INSERT INTO PG_LSN_TBL VALUES ('G/0');
-ERROR:  invalid input syntax for transaction log location: "G/0"
+ERROR:  invalid input syntax for type pg_lsn: "G/0"
 LINE 1: INSERT INTO PG_LSN_TBL VALUES ('G/0');
                                        ^
 INSERT INTO PG_LSN_TBL VALUES ('-1/0');
-ERROR:  invalid input syntax for transaction log location: "-1/0"
+ERROR:  invalid input syntax for type pg_lsn: "-1/0"
 LINE 1: INSERT INTO PG_LSN_TBL VALUES ('-1/0');
                                        ^
 INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
-ERROR:  invalid input syntax for transaction log location: " 0/12345678"
+ERROR:  invalid input syntax for type pg_lsn: " 0/12345678"
 LINE 1: INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
                                        ^
 INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
-ERROR:  invalid input syntax for transaction log location: "ABCD/"
+ERROR:  invalid input syntax for type pg_lsn: "ABCD/"
 LINE 1: INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
                                        ^
 INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
-ERROR:  invalid input syntax for transaction log location: "/ABCD"
+ERROR:  invalid input syntax for type pg_lsn: "/ABCD"
 LINE 1: INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
                                        ^
 DROP TABLE PG_LSN_TBL;