Subject: Re: Consolidate ItemPointer to Datum conversion functions

From: Heikki Linnakangas <[email-masked]>
Date: Mon, 6 Feb 2023 11:11:54 +0100

On 06/02/2023 11:54, Peter Eisentraut wrote:
> Instead of defining the same set of macros several times, define it once
> in an appropriate header file.  In passing, convert to inline functions.

Looks good to me. Did you consider moving PG_GETARG_ITEMPOINTER and 
PG_RETURN_ITEMPOINTER, too? They're only used in tid.c, but for most 
datatypes, we define the PG_GETARG and PG_RETURN macros in the same 
header file as the the Datum conversion functions.

- Heikki

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 9 Feb 2023 09:33:16 +0100

> header file as the the Datum conversion functions.

Yeah that makes sense.  Here is an updated patch for that.
From 47b316eb6cfa50412d81fd183a8b54e0a104a685 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email-masked]>
Date: Thu, 9 Feb 2023 09:23:20 +0100
Subject: [PATCH v2] Consolidate ItemPointer to Datum conversion functions

Instead of defining the same set of macros several times, define it
once in an appropriate header file.  In passing, convert to inline
functions.

Discussion: https://www.postgresql.org/message-id/flat/844dd4c5-e5a1-3df1-bfaf-d1e1c2a16e45%40enterprisedb.com
---
 contrib/pageinspect/btreefuncs.c |  2 --
 contrib/pageinspect/ginfuncs.c   |  3 ---
 contrib/pageinspect/gistfuncs.c  |  2 --
 src/backend/utils/adt/tid.c      |  5 -----
 src/include/storage/itemptr.h    | 20 ++++++++++++++++++++
 5 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index e4e5dc3c81..9cdc8e182b 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -50,8 +50,6 @@ PG_FUNCTION_INFO_V1(bt_multi_page_stats);
 
 #define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
 #define IS_BTREE(r) ((r)->rd_rel->relam == BTREE_AM_OID)
-#define DatumGetItemPointer(X)	 ((ItemPointer) DatumGetPointer(X))
-#define ItemPointerGetDatum(X)	 PointerGetDatum(X)
 
 /* ------------------------------------------------
  * structure for single btree page statistics
diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c
index efaa47e86d..0f846988df 100644
--- a/contrib/pageinspect/ginfuncs.c
+++ b/contrib/pageinspect/ginfuncs.c
@@ -21,9 +21,6 @@
 #include "utils/builtins.h"
 #include "utils/rel.h"
 
-#define DatumGetItemPointer(X)	 ((ItemPointer) DatumGetPointer(X))
-#define ItemPointerGetDatum(X)	 PointerGetDatum(X)
-
 
 PG_FUNCTION_INFO_V1(gin_metapage_info);
 PG_FUNCTION_INFO_V1(gin_page_opaque_info);
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index 3a947c82af..100697814d 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -31,8 +31,6 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 
 #define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
 
-#define ItemPointerGetDatum(X)	 PointerGetDatum(X)
-
 
 Datum
 gist_page_opaque_info(PG_FUNCTION_ARGS)
diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c
index 251219a1ef..77fb74ab0c 100644
--- a/src/backend/utils/adt/tid.c
+++ b/src/backend/utils/adt/tid.c
@@ -37,11 +37,6 @@
 #include "utils/varlena.h"

-#define DatumGetItemPointer(X)	 ((ItemPointer) DatumGetPointer(X))
-#define ItemPointerGetDatum(X)	 PointerGetDatum(X)
-#define PG_GETARG_ITEMPOINTER(n) DatumGetItemPointer(PG_GETARG_DATUM(n))
-#define PG_RETURN_ITEMPOINTER(x) return ItemPointerGetDatum(x)
-
 #define LDELIM			'('
 #define RDELIM			')'
 #define DELIM			','
diff --git a/src/include/storage/itemptr.h b/src/include/storage/itemptr.h
index 354e50e68b..fafefa14cd 100644
--- a/src/include/storage/itemptr.h
+++ b/src/include/storage/itemptr.h
@@ -222,4 +222,24 @@ extern int32 ItemPointerCompare(ItemPointer arg1, ItemPointer arg2);
 extern void ItemPointerInc(ItemPointer pointer);
 extern void ItemPointerDec(ItemPointer pointer);
 
+/* ----------------
+ *		Datum conversion functions
+ * ----------------
+ */
+
+static inline ItemPointer
+DatumGetItemPointer(Datum X)
+{
+	return (ItemPointer) DatumGetPointer(X);
+}
+
+static inline Datum
+ItemPointerGetDatum(const ItemPointerData *X)
+{
+	return PointerGetDatum(X);
+}
+
+#define PG_GETARG_ITEMPOINTER(n) DatumGetItemPointer(PG_GETARG_DATUM(n))
+#define PG_RETURN_ITEMPOINTER(x) return ItemPointerGetDatum(x)
+
 #endif							/* ITEMPTR_H */
-- 
2.39.1

---

From: Peter Eisentraut <[email-masked]>
Date: Mon, 13 Feb 2023 10:26:58 +0100

> Yeah that makes sense.  Here is an updated patch for that.

committed