> > You're ignoring the xid use-case, for which DatumGetUInt32 actually is > > the right thing. > > There is DatumGetTransactionId() which should be used instead. > That made me search if there's PG_GETARG_TRANSACTIONID() and yes it's > there but only defined in xid.c. So pg_xact_commit_timestamp(), > pg_xact_commit_timestamp_origin() and pg_get_multixact_members() use > PG_GETARG_UNIT32. IMO those should be changed to use > PG_GETARG_TRANSACTIONID. That would require moving > PG_GETARG_TRANSACTIONID somewhere outside xid.c; may be fmgr.h where > other PG_GETARG_* are.
Hmm, yeah, I think this would be a good idea.
The patch 0003 does that.
> get_raw_page() also does similar thing but the effect is not as dangerous > SELECT octet_length(get_raw_page('test1', 'main', -1)) AS main_1; > ERROR: block number 4294967295 is out of range for relation "test1" > Similarly for bt_page_stats() and bt_page_items()
Hmm, but page numbers above signed INT_MAX are valid. So this would prevent reading all legitimate pages past that.
According to https://www.postgresql.org/docs/12/datatype-numeric.html, these functions shouldn't be accepting values higher than INT_MAX since it's outside the integer data type range. But may be it's a convenient way to avoid using bigint. Anyway those changes are separate in 0002 patch which can be discarded as a whole. But for now I am keeping it in the bunch.
