Re: Improper use about DatumGetInt32

On Fri, 16 Oct 2020 at 19:26, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
wrote:

> On 2020-Sep-23, Ashutosh Bapat wrote:
>
> > > 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.

--
Best Wishes,
Ashutosh