Hi all, (Aleksander in CC, who has authored some of them in pageinspect.) While looking at the business in [1], I have noticed a bunch of areas in the code base where the *GetDatum() macros used do not match with the data type of the SQL functions they relate to, leading me to the attached.
I am creating a new thread for clarity, as the patch was hidden in the middle of a different discussion. When it comes to the numbers touched here, the signedness should not matter, so this most likely a cleanup worth doing for v20, not v19. Thanks, [1]: https://www.postgresql.org/message-id/CAJ7c6TMcGu8qmRe1gZfJ-gOzVnZq-t=fwn-uuystx1w6zyy...@mail.gmail.com -- Michael
From b818284605c1a4b533943f651f9aea9ae4a26d02 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Thu, 30 Apr 2026 14:30:12 +0900 Subject: [PATCH] Fix more Datum conversion inconsistencies This is a continuation of the work done in the previous commit, that has undone the work of 6dcfac9696cb. The *GetDatum() macros for output should match with what the SQL functions use as DatumGet*() in input. Aleksander has spotted some of the areas patched here, for pageinspect. I have spotted the rest while digging into the state of the tree. There is no actual behavior change after this commit, since all the affected values are small enough that the signed bit is never used. Author: Aleksander Alekseev <[email protected]> Author: Michael Paquier <[email protected]> --- src/backend/access/gin/ginlogic.c | 6 +++--- src/backend/access/transam/xlogfuncs.c | 2 +- src/backend/catalog/pg_proc.c | 4 ++-- src/backend/utils/adt/lockfuncs.c | 10 +++++----- contrib/pageinspect/brinfuncs.c | 8 ++++---- contrib/pageinspect/btreefuncs.c | 2 +- contrib/pageinspect/ginfuncs.c | 2 +- contrib/pageinspect/gistfuncs.c | 4 ++-- contrib/pageinspect/heapfuncs.c | 18 +++++++++--------- contrib/pageinspect/rawpage.c | 14 +++++++------- contrib/pg_logicalinspect/pg_logicalinspect.c | 8 ++++---- contrib/pg_walinspect/pg_walinspect.c | 14 +++++++------- contrib/pgstattuple/pgstatindex.c | 2 +- 13 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/backend/access/gin/ginlogic.c b/src/backend/access/gin/ginlogic.c index e851d49f74de..e59d655da623 100644 --- a/src/backend/access/gin/ginlogic.c +++ b/src/backend/access/gin/ginlogic.c @@ -75,7 +75,7 @@ directBoolConsistentFn(GinScanKey key) PointerGetDatum(key->entryRes), UInt16GetDatum(key->strategy), key->query, - UInt32GetDatum(key->nuserentries), + Int32GetDatum(key->nuserentries), PointerGetDatum(key->extra_data), PointerGetDatum(&key->recheckCurItem), PointerGetDatum(key->queryValues), @@ -93,7 +93,7 @@ directTriConsistentFn(GinScanKey key) PointerGetDatum(key->entryRes), UInt16GetDatum(key->strategy), key->query, - UInt32GetDatum(key->nuserentries), + Int32GetDatum(key->nuserentries), PointerGetDatum(key->extra_data), PointerGetDatum(key->queryValues), PointerGetDatum(key->queryCategories))); @@ -114,7 +114,7 @@ shimBoolConsistentFn(GinScanKey key) PointerGetDatum(key->entryRes), UInt16GetDatum(key->strategy), key->query, - UInt32GetDatum(key->nuserentries), + Int32GetDatum(key->nuserentries), PointerGetDatum(key->extra_data), PointerGetDatum(key->queryValues), PointerGetDatum(key->queryCategories))); diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index 0f5979691e6b..207da27d0347 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -448,7 +448,7 @@ pg_walfile_name_offset(PG_FUNCTION_ARGS) */ xrecoff = XLogSegmentOffset(locationpoint, wal_segment_size); - values[1] = UInt32GetDatum(xrecoff); + values[1] = Int32GetDatum(xrecoff); isnull[1] = false; /* diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 5df4b3f7a91e..4c6dfb5ca908 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -342,8 +342,8 @@ ProcedureCreate(const char *procedureName, values[Anum_pg_proc_proretset - 1] = BoolGetDatum(returnsSet); values[Anum_pg_proc_provolatile - 1] = CharGetDatum(volatility); values[Anum_pg_proc_proparallel - 1] = CharGetDatum(parallel); - values[Anum_pg_proc_pronargs - 1] = UInt16GetDatum(parameterCount); - values[Anum_pg_proc_pronargdefaults - 1] = UInt16GetDatum(list_length(parameterDefaults)); + values[Anum_pg_proc_pronargs - 1] = Int16GetDatum(parameterCount); + values[Anum_pg_proc_pronargdefaults - 1] = Int16GetDatum(list_length(parameterDefaults)); values[Anum_pg_proc_prorettype - 1] = ObjectIdGetDatum(returnType); values[Anum_pg_proc_proargtypes - 1] = PointerGetDatum(parameterTypes); if (allParameterTypes != PointerGetDatum(NULL)) diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c index dc58e9cb0a6d..949b242322cb 100644 --- a/src/backend/utils/adt/lockfuncs.c +++ b/src/backend/utils/adt/lockfuncs.c @@ -271,7 +271,7 @@ pg_lock_status(PG_FUNCTION_ARGS) case LOCKTAG_PAGE: values[1] = ObjectIdGetDatum(instance->locktag.locktag_field1); values[2] = ObjectIdGetDatum(instance->locktag.locktag_field2); - values[3] = UInt32GetDatum(instance->locktag.locktag_field3); + values[3] = Int32GetDatum(instance->locktag.locktag_field3); nulls[4] = true; nulls[5] = true; nulls[6] = true; @@ -282,8 +282,8 @@ pg_lock_status(PG_FUNCTION_ARGS) case LOCKTAG_TUPLE: values[1] = ObjectIdGetDatum(instance->locktag.locktag_field1); values[2] = ObjectIdGetDatum(instance->locktag.locktag_field2); - values[3] = UInt32GetDatum(instance->locktag.locktag_field3); - values[4] = UInt16GetDatum(instance->locktag.locktag_field4); + values[3] = Int32GetDatum(instance->locktag.locktag_field3); + values[4] = Int16GetDatum(instance->locktag.locktag_field4); nulls[5] = true; nulls[6] = true; nulls[7] = true; @@ -402,12 +402,12 @@ pg_lock_status(PG_FUNCTION_ARGS) values[1] = ObjectIdGetDatum(GET_PREDICATELOCKTARGETTAG_DB(*predTag)); values[2] = ObjectIdGetDatum(GET_PREDICATELOCKTARGETTAG_RELATION(*predTag)); if (lockType == PREDLOCKTAG_TUPLE) - values[4] = UInt16GetDatum(GET_PREDICATELOCKTARGETTAG_OFFSET(*predTag)); + values[4] = Int16GetDatum(GET_PREDICATELOCKTARGETTAG_OFFSET(*predTag)); else nulls[4] = true; if ((lockType == PREDLOCKTAG_TUPLE) || (lockType == PREDLOCKTAG_PAGE)) - values[3] = UInt32GetDatum(GET_PREDICATELOCKTARGETTAG_PAGE(*predTag)); + values[3] = Int32GetDatum(GET_PREDICATELOCKTARGETTAG_PAGE(*predTag)); else nulls[3] = true; diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c index 309b9522f902..c64124b5b020 100644 --- a/contrib/pageinspect/brinfuncs.c +++ b/contrib/pageinspect/brinfuncs.c @@ -246,7 +246,7 @@ brin_page_items(PG_FUNCTION_ARGS) if (unusedItem) { - values[0] = UInt16GetDatum(offset); + values[0] = Int32GetDatum(offset); nulls[1] = true; nulls[2] = true; nulls[3] = true; @@ -259,7 +259,7 @@ brin_page_items(PG_FUNCTION_ARGS) { int att = attno - 1; - values[0] = UInt16GetDatum(offset); + values[0] = Int32GetDatum(offset); switch (TupleDescAttr(rsinfo->setDesc, 1)->atttypid) { case INT8OID: @@ -267,12 +267,12 @@ brin_page_items(PG_FUNCTION_ARGS) break; case INT4OID: /* support for old extension version */ - values[1] = UInt32GetDatum(dtup->bt_blkno); + values[1] = Int32GetDatum(dtup->bt_blkno); break; default: elog(ERROR, "incorrect output types"); } - values[2] = UInt16GetDatum(attno); + values[2] = Int32GetDatum(attno); values[3] = BoolGetDatum(dtup->bt_columns[att].bv_allnulls); values[4] = BoolGetDatum(dtup->bt_columns[att].bv_hasnulls); values[5] = BoolGetDatum(dtup->bt_placeholder); diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index 62c905c6e7c2..3381e7cc2a74 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -509,7 +509,7 @@ bt_page_print_tuples(ua_page_items *uargs) memset(nulls, 0, sizeof(nulls)); values[j++] = Int16GetDatum(offset); values[j++] = ItemPointerGetDatum(&itup->t_tid); - values[j++] = Int32GetDatum((int) IndexTupleSize(itup)); + values[j++] = Int16GetDatum(IndexTupleSize(itup)); values[j++] = BoolGetDatum(IndexTupleHasNulls(itup)); values[j++] = BoolGetDatum(IndexTupleHasVarwidths(itup)); diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c index ebcc2b3db5c7..e4461db7f0a2 100644 --- a/contrib/pageinspect/ginfuncs.c +++ b/contrib/pageinspect/ginfuncs.c @@ -258,7 +258,7 @@ gin_leafpage_items(PG_FUNCTION_ARGS) memset(nulls, 0, sizeof(nulls)); values[0] = ItemPointerGetDatum(&cur->first); - values[1] = UInt16GetDatum(cur->nbytes); + values[1] = Int16GetDatum(cur->nbytes); /* build an array of decoded item pointers */ tids = ginPostingListDecode(cur, &ndecoded); diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c index 23a4b49771d4..8f127d41ec49 100644 --- a/contrib/pageinspect/gistfuncs.c +++ b/contrib/pageinspect/gistfuncs.c @@ -179,7 +179,7 @@ gist_page_items_bytea(PG_FUNCTION_ARGS) values[0] = Int16GetDatum(offset); values[1] = ItemPointerGetDatum(&itup->t_tid); - values[2] = Int32GetDatum((int) IndexTupleSize(itup)); + values[2] = Int16GetDatum(IndexTupleSize(itup)); tuple_bytea = (bytea *) palloc(tuple_len + VARHDRSZ); SET_VARSIZE(tuple_bytea, tuple_len + VARHDRSZ); @@ -286,7 +286,7 @@ gist_page_items(PG_FUNCTION_ARGS) values[0] = Int16GetDatum(offset); values[1] = ItemPointerGetDatum(&itup->t_tid); - values[2] = Int32GetDatum((int) IndexTupleSize(itup)); + values[2] = Int16GetDatum(IndexTupleSize(itup)); values[3] = BoolGetDatum(ItemIdIsDead(id)); if (index_columns) diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c index 4f0f3bd53e74..7be4770dc84b 100644 --- a/contrib/pageinspect/heapfuncs.c +++ b/contrib/pageinspect/heapfuncs.c @@ -189,10 +189,10 @@ heap_page_items(PG_FUNCTION_ARGS) lp_flags = ItemIdGetFlags(id); lp_len = ItemIdGetLength(id); - values[0] = UInt16GetDatum(inter_call_data->offset); - values[1] = UInt16GetDatum(lp_offset); - values[2] = UInt16GetDatum(lp_flags); - values[3] = UInt16GetDatum(lp_len); + values[0] = Int16GetDatum(inter_call_data->offset); + values[1] = Int16GetDatum(lp_offset); + values[2] = Int16GetDatum(lp_flags); + values[3] = Int16GetDatum(lp_len); /* * We do just enough validity checking to make sure we don't reference @@ -209,13 +209,13 @@ heap_page_items(PG_FUNCTION_ARGS) /* Extract information from the tuple header */ tuphdr = (HeapTupleHeader) PageGetItem(page, id); - values[4] = UInt32GetDatum(HeapTupleHeaderGetRawXmin(tuphdr)); - values[5] = UInt32GetDatum(HeapTupleHeaderGetRawXmax(tuphdr)); + values[4] = TransactionIdGetDatum(HeapTupleHeaderGetRawXmin(tuphdr)); + values[5] = TransactionIdGetDatum(HeapTupleHeaderGetRawXmax(tuphdr)); /* shared with xvac */ - values[6] = UInt32GetDatum(HeapTupleHeaderGetRawCommandId(tuphdr)); + values[6] = Int32GetDatum(HeapTupleHeaderGetRawCommandId(tuphdr)); values[7] = PointerGetDatum(&tuphdr->t_ctid); - values[8] = UInt32GetDatum(tuphdr->t_infomask2); - values[9] = UInt32GetDatum(tuphdr->t_infomask); + values[8] = Int32GetDatum(tuphdr->t_infomask2); + values[9] = Int32GetDatum(tuphdr->t_infomask); values[10] = UInt8GetDatum(tuphdr->t_hoff); /* diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index f3996dc39fcd..d136593edb26 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -284,8 +284,8 @@ page_header(PG_FUNCTION_ARGS) } else values[0] = LSNGetDatum(lsn); - values[1] = UInt16GetDatum(pageheader->pd_checksum); - values[2] = UInt16GetDatum(pageheader->pd_flags); + values[1] = Int16GetDatum(pageheader->pd_checksum); + values[2] = Int16GetDatum(pageheader->pd_flags); /* pageinspect >= 1.10 uses int4 instead of int2 for those fields */ switch (TupleDescAttr(tupdesc, 3)->atttypid) @@ -294,10 +294,10 @@ page_header(PG_FUNCTION_ARGS) Assert(TupleDescAttr(tupdesc, 4)->atttypid == INT2OID && TupleDescAttr(tupdesc, 5)->atttypid == INT2OID && TupleDescAttr(tupdesc, 6)->atttypid == INT2OID); - values[3] = UInt16GetDatum(pageheader->pd_lower); - values[4] = UInt16GetDatum(pageheader->pd_upper); - values[5] = UInt16GetDatum(pageheader->pd_special); - values[6] = UInt16GetDatum(PageGetPageSize(page)); + values[3] = Int16GetDatum(pageheader->pd_lower); + values[4] = Int16GetDatum(pageheader->pd_upper); + values[5] = Int16GetDatum(pageheader->pd_special); + values[6] = Int16GetDatum(PageGetPageSize(page)); break; case INT4OID: Assert(TupleDescAttr(tupdesc, 4)->atttypid == INT4OID && @@ -313,7 +313,7 @@ page_header(PG_FUNCTION_ARGS) break; } - values[7] = UInt16GetDatum(PageGetPageLayoutVersion(page)); + values[7] = Int16GetDatum(PageGetPageLayoutVersion(page)); values[8] = TransactionIdGetDatum(pageheader->pd_prune_xid); /* Build and return the tuple. */ diff --git a/contrib/pg_logicalinspect/pg_logicalinspect.c b/contrib/pg_logicalinspect/pg_logicalinspect.c index 9420fa5e0c7c..389394de66b3 100644 --- a/contrib/pg_logicalinspect/pg_logicalinspect.c +++ b/contrib/pg_logicalinspect/pg_logicalinspect.c @@ -117,9 +117,9 @@ pg_get_logical_snapshot_meta(PG_FUNCTION_ARGS) /* Validate and restore the snapshot to 'ondisk' */ SnapBuildRestoreSnapshot(&ondisk, lsn, CurrentMemoryContext, false); - values[i++] = UInt32GetDatum(ondisk.magic); + values[i++] = Int32GetDatum(ondisk.magic); values[i++] = Int64GetDatum((int64) ondisk.checksum); - values[i++] = UInt32GetDatum(ondisk.version); + values[i++] = Int32GetDatum(ondisk.version); Assert(i == PG_GET_LOGICAL_SNAPSHOT_META_COLS); @@ -163,7 +163,7 @@ pg_get_logical_snapshot_info(PG_FUNCTION_ARGS) values[i++] = LSNGetDatum(ondisk.builder.last_serialized_snapshot); values[i++] = TransactionIdGetDatum(ondisk.builder.next_phase_at); - values[i++] = UInt32GetDatum(ondisk.builder.committed.xcnt); + values[i++] = Int32GetDatum(ondisk.builder.committed.xcnt); if (ondisk.builder.committed.xcnt > 0) { Datum *arrayelems; @@ -180,7 +180,7 @@ pg_get_logical_snapshot_info(PG_FUNCTION_ARGS) else nulls[i++] = true; - values[i++] = UInt32GetDatum(ondisk.builder.catchange.xcnt); + values[i++] = Int32GetDatum(ondisk.builder.catchange.xcnt); if (ondisk.builder.catchange.xcnt > 0) { Datum *arrayelems; diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c index 4cf6e41e2f5c..a172f9e2b407 100644 --- a/contrib/pg_walinspect/pg_walinspect.c +++ b/contrib/pg_walinspect/pg_walinspect.c @@ -230,9 +230,9 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values, values[i++] = TransactionIdGetDatum(XLogRecGetXid(record)); values[i++] = CStringGetTextDatum(desc.rm_name); values[i++] = CStringGetTextDatum(record_type); - values[i++] = UInt32GetDatum(XLogRecGetTotalLen(record)); - values[i++] = UInt32GetDatum(XLogRecGetDataLen(record)); - values[i++] = UInt32GetDatum(fpi_len); + values[i++] = Int32GetDatum(XLogRecGetTotalLen(record)); + values[i++] = Int32GetDatum(XLogRecGetDataLen(record)); + values[i++] = Int32GetDatum(fpi_len); if (rec_desc.len > 0) values[i++] = CStringGetTextDatum(rec_desc.data); @@ -357,10 +357,10 @@ GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record, * record_length, main_data_length, block_data_len, and * block_fpi_length outputs */ - values[i++] = UInt32GetDatum(XLogRecGetTotalLen(record)); - values[i++] = UInt32GetDatum(XLogRecGetDataLen(record)); - values[i++] = UInt32GetDatum(block_data_len); - values[i++] = UInt32GetDatum(block_fpi_len); + values[i++] = Int32GetDatum(XLogRecGetTotalLen(record)); + values[i++] = Int32GetDatum(XLogRecGetDataLen(record)); + values[i++] = Int32GetDatum(block_data_len); + values[i++] = Int32GetDatum(block_fpi_len); /* block_fpi_info (text array) output */ if (block_fpi_info) diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index 3a3f2637bd95..8951ad0aac47 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -586,7 +586,7 @@ pgstatginindex_internal(Oid relid, FunctionCallInfo fcinfo) elog(ERROR, "return type must be a row type"); values[0] = Int32GetDatum(stats.version); - values[1] = UInt32GetDatum(stats.pending_pages); + values[1] = Int32GetDatum(stats.pending_pages); values[2] = Int64GetDatum(stats.pending_tuples); /* -- 2.54.0
signature.asc
Description: PGP signature
