Use correct macro for accessing offset numbers.

Started by Kirill Reshkeabout 17 hours ago4 messages

reshkekirill@gmail.com

about 17 hours ago

1 attachment(s)

Hi hackers!

While working on pageinspect support for GIN and SpGiST (welcome to
review them [0]/messages/by-id/CALdSSPiN13n7feQcY0WCmq8jzxjwqhNrt1E=g=g6aZANyE_OoQ@mail.gmail.com & [1]/messages/by-id/CALdSSPhbAQbFtjK0nT8_G5GsXmsSEVx8J735Ga+ZxLp9osHcRA@mail.gmail.com), I spotted $subi.

PFA trivial patch that uses UInt16GetDatum for OffsetNumber rather
than Int16GetDatum

[0]: /messages/by-id/CALdSSPiN13n7feQcY0WCmq8jzxjwqhNrt1E=g=g6aZANyE_OoQ@mail.gmail.com
[1]: /messages/by-id/CALdSSPhbAQbFtjK0nT8_G5GsXmsSEVx8J735Ga+ZxLp9osHcRA@mail.gmail.com

--
Best regards,
Kirill Reshke

Attachments:

v1-0001-Use-correct-macro-for-accessing-offset-numbers.patchapplication/octet-stream; name=v1-0001-Use-correct-macro-for-accessing-offset-numbers.patchDownload

From 803a8aac4656c320362cad6c55508320983b3c57 Mon Sep 17 00:00:00 2001
From: reshke <reshke@double.cloud>
Date: Sun, 11 Jan 2026 11:17:13 +0000
Subject: [PATCH v1] Use correct macro for accessing offset numbers.

---
 contrib/pageinspect/btreefuncs.c | 2 +-
 contrib/pageinspect/gistfuncs.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 62c905c6e7c..0585b7cee40 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -507,7 +507,7 @@ bt_page_print_tuples(ua_page_items *uargs)
 
 	j = 0;
 	memset(nulls, 0, sizeof(nulls));
-	values[j++] = Int16GetDatum(offset);
+	values[j++] = UInt16GetDatum(offset);
 	values[j++] = ItemPointerGetDatum(&itup->t_tid);
 	values[j++] = Int32GetDatum((int) IndexTupleSize(itup));
 	values[j++] = BoolGetDatum(IndexTupleHasNulls(itup));
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index 60a4b240302..9b7e3cec882 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -175,7 +175,7 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 
 		memset(nulls, 0, sizeof(nulls));
 
-		values[0] = Int16GetDatum(offset);
+		values[0] = UInt16GetDatum(offset);
 		values[1] = ItemPointerGetDatum(&itup->t_tid);
 		values[2] = Int32GetDatum((int) IndexTupleSize(itup));
 
@@ -282,7 +282,7 @@ gist_page_items(PG_FUNCTION_ARGS)
 
 		memset(nulls, 0, sizeof(nulls));
 
-		values[0] = Int16GetDatum(offset);
+		values[0] = UInt16GetDatum(offset);
 		values[1] = ItemPointerGetDatum(&itup->t_tid);
 		values[2] = Int32GetDatum((int) IndexTupleSize(itup));
 		values[3] = BoolGetDatum(ItemIdIsDead(id));
-- 
2.43.0

Roman Khapov

rkhapov@yandex-team.ru

about 16 hours ago

In reply to: Kirill Reshke (#1)

Re: Use correct macro for accessing offset numbers.

On 11 Jan 2026, at 16:21, Kirill Reshke <reshkekirill@gmail.com> wrote:

Hi hackers!

PFA trivial patch that uses UInt16GetDatum for OffsetNumber rather
than Int16GetDatum

Hi!

LGTM, should we check another places of offset number conversations to Datum
as part of this thread?

--
Best regards,
Roman Khapov

Kirill Reshke

reshkekirill@gmail.com

about 16 hours ago

In reply to: Roman Khapov (#2)

2 attachment(s)

Re: Use correct macro for accessing offset numbers.

On Sun, 11 Jan 2026 at 16:41, Roman Khapov <rkhapov@yandex-team.ru> wrote:

should we check another places of offset number conversations to Datum
as part of this thread?

Maybe, I have stopped some more cases, in v2-0001

--
Best regards,
Kirill Reshke

Attachments:

v1-0001-Use-correct-macro-for-accessing-offset-numbers.patchapplication/octet-stream; name=v1-0001-Use-correct-macro-for-accessing-offset-numbers.patchDownload

From 803a8aac4656c320362cad6c55508320983b3c57 Mon Sep 17 00:00:00 2001
From: reshke <reshke@double.cloud>
Date: Sun, 11 Jan 2026 11:17:13 +0000
Subject: [PATCH v1] Use correct macro for accessing offset numbers.

---
 contrib/pageinspect/btreefuncs.c | 2 +-
 contrib/pageinspect/gistfuncs.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 62c905c6e7c..0585b7cee40 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -507,7 +507,7 @@ bt_page_print_tuples(ua_page_items *uargs)
 
 	j = 0;
 	memset(nulls, 0, sizeof(nulls));
-	values[j++] = Int16GetDatum(offset);
+	values[j++] = UInt16GetDatum(offset);
 	values[j++] = ItemPointerGetDatum(&itup->t_tid);
 	values[j++] = Int32GetDatum((int) IndexTupleSize(itup));
 	values[j++] = BoolGetDatum(IndexTupleHasNulls(itup));
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index 60a4b240302..9b7e3cec882 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -175,7 +175,7 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 
 		memset(nulls, 0, sizeof(nulls));
 
-		values[0] = Int16GetDatum(offset);
+		values[0] = UInt16GetDatum(offset);
 		values[1] = ItemPointerGetDatum(&itup->t_tid);
 		values[2] = Int32GetDatum((int) IndexTupleSize(itup));
 
@@ -282,7 +282,7 @@ gist_page_items(PG_FUNCTION_ARGS)
 
 		memset(nulls, 0, sizeof(nulls));
 
-		values[0] = Int16GetDatum(offset);
+		values[0] = UInt16GetDatum(offset);
 		values[1] = ItemPointerGetDatum(&itup->t_tid);
 		values[2] = Int32GetDatum((int) IndexTupleSize(itup));
 		values[3] = BoolGetDatum(ItemIdIsDead(id));
-- 
2.43.0

v2-0001-Use-UInt16GetDatum-for-stategy-number-access.patchapplication/octet-stream; name=v2-0001-Use-UInt16GetDatum-for-stategy-number-access.patchDownload

From f6104a9e39cb14c725ae46168044822e51ddbbc3 Mon Sep 17 00:00:00 2001
From: reshke <reshke@double.cloud>
Date: Sun, 11 Jan 2026 11:57:04 +0000
Subject: [PATCH v2] Use UInt16GetDatum for stategy number access.

---
 contrib/pg_buffercache/pg_buffercache_pages.c | 2 +-
 src/backend/access/brin/brin_inclusion.c      | 2 +-
 src/backend/access/brin/brin_minmax.c         | 2 +-
 src/backend/access/brin/brin_minmax_multi.c   | 2 +-
 src/backend/access/gist/gistget.c             | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index 0c58e4b265c..b682dca658b 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -276,7 +276,7 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
 			nulls[5] = false;
 			values[6] = BoolGetDatum(fctx->record[i].isdirty);
 			nulls[6] = false;
-			values[7] = Int16GetDatum(fctx->record[i].usagecount);
+			values[7] = UInt16GetDatum(fctx->record[i].usagecount);
 			nulls[7] = false;
 			/* unused for v1.0 callers, but the array is always long enough */
 			values[8] = Int32GetDatum(fctx->record[i].pinning_backends);
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c
index 08890a3d009..5a2058d9aad 100644
--- a/src/backend/access/brin/brin_inclusion.c
+++ b/src/backend/access/brin/brin_inclusion.c
@@ -641,7 +641,7 @@ inclusion_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
 		tuple = SearchSysCache4(AMOPSTRATEGY, ObjectIdGetDatum(opfamily),
 								ObjectIdGetDatum(attr->atttypid),
 								ObjectIdGetDatum(subtype),
-								Int16GetDatum(strategynum));
+								UInt16GetDatum(strategynum));
 
 		if (!HeapTupleIsValid(tuple))
 			elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 9d4e47b4dc0..73201029371 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -294,7 +294,7 @@ minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
 		tuple = SearchSysCache4(AMOPSTRATEGY, ObjectIdGetDatum(opfamily),
 								ObjectIdGetDatum(attr->atttypid),
 								ObjectIdGetDatum(subtype),
-								Int16GetDatum(strategynum));
+								UInt16GetDatum(strategynum));
 
 		if (!HeapTupleIsValid(tuple))
 			elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 6b86b1fd889..688ca9f2dbb 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -2932,7 +2932,7 @@ minmax_multi_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
 		tuple = SearchSysCache4(AMOPSTRATEGY, ObjectIdGetDatum(opfamily),
 								ObjectIdGetDatum(attr->atttypid),
 								ObjectIdGetDatum(subtype),
-								Int16GetDatum(strategynum));
+								UInt16GetDatum(strategynum));
 		if (!HeapTupleIsValid(tuple))
 			elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
 				 strategynum, attr->atttypid, subtype, opfamily);
diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
index 6d05a5fdc34..d6de8e954e8 100644
--- a/src/backend/access/gist/gistget.c
+++ b/src/backend/access/gist/gistget.c
@@ -222,7 +222,7 @@ gistindex_keytest(IndexScanDesc scan,
 									 key->sk_collation,
 									 PointerGetDatum(&de),
 									 key->sk_argument,
-									 Int16GetDatum(key->sk_strategy),
+									 UInt16GetDatum(key->sk_strategy),
 									 ObjectIdGetDatum(key->sk_subtype),
 									 PointerGetDatum(&recheck));
 
@@ -286,7 +286,7 @@ gistindex_keytest(IndexScanDesc scan,
 									 key->sk_collation,
 									 PointerGetDatum(&de),
 									 key->sk_argument,
-									 Int16GetDatum(key->sk_strategy),
+									 UInt16GetDatum(key->sk_strategy),
 									 ObjectIdGetDatum(key->sk_subtype),
 									 PointerGetDatum(&recheck));
 			*recheck_distances_p |= recheck;
-- 
2.43.0

Michael Paquier

michael@paquier.xyz

about 5 hours ago

In reply to: Kirill Reshke (#3)

Re: Use correct macro for accessing offset numbers.

On Sun, Jan 11, 2026 at 04:58:39PM +0500, Kirill Reshke wrote:

Maybe, I have stopped some more cases, in v2-0001

Right. It's true that we could be more consistent for all these based
on their base type, some of them, particularly in the GIN code now,
caring about using the correct macro. It may be a good occasion to
double-check the whole tree for similar holes based on unsigned types.
--
Michael