From 694e3d139a9d090c58494428bebfadad216419da Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 19 Feb 2014 10:06:59 -0500 Subject: [PATCH] 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. --- src/backend/utils/adt/pg_lsn.c | 38 ++++++++++++++-------------- src/include/catalog/pg_type.h | 2 +- src/include/fmgr.h | 2 -- src/include/postgres.h | 14 ---------- src/include/utils/pg_lsn.h | 7 +++++ src/test/regress/expected/pg_lsn.out | 10 ++++---- 6 files changed, 32 insertions(+), 41 deletions(-) diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c index 45f5131091..e2b528a243 100644 --- 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 db18a23bf6..8ea9ceb1b8 100644 --- 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 dd4c672baf..aed81cdc26 100644 --- 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 c8b311fa22..a8a206d988 100644 --- 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 dc2193006e..981fcd6fa8 100644 --- 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 8cfe28e4b8..01d298397b 100644 --- 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;