Re: [COMMITTERS] pgsql: pageinspect: Try to fix some bugs in previous commit.

>>>> I think UInt32GetDatum(metad->hashm_procid) looks fine, the reason
>>>> being 'hashm_procid' is defined as 32-bit unsigned integer but then we
>>>> may have to define procid as int8 in SQL function.
>>>
>>> No, you're wrong. The GetDatum you choose macro has to match the SQL
>>> type, not the type of the variable that you're passing to it. For
>>> example, if you've got an "int" in the code and the SQL type is
>>> "int8", you have to use Int64GetDatum, not Int32GetDatum. Otherwise,
>>> stuff breaks, because on some systems 64-bit integers are passed by
>>> reference, not by value, so the representation that Int32GetDatum
>>> produces isn't valid when interpreted by DatumGetInt64 later on. The
>>> latter is expecting a pointer, but the former didn't produce one.
>>>
>>
>> Thank you very much for detailed information and explanation. It is
>> really very helpful and understandable. But, As per your explanation,
>> GetDatum we choose need to match the SQL type, not the type of the
>> variable used in code and I do not see any unsigned integer SQL type
>> in PostgreSQL then I am just wondering on why do we have
>> UInt32GetDatum or UInt64GetDatum macros.
>
> That's a pretty good question. UInt64GetDatum is used in exactly one
> place (exec_stmt_getdiag) and there's really no reason why
> Int64GetDatum wouldn't be more appropriate. UInt32GetDatum is used in
> a few more places, and some of those are used for hash values which
> are not exposed at the SQL level so they might be legitimate, but
> others like the ones in pageinspect look like fuzzy thinking that has
> only survived because it happens not to break anything. I suppose if
> we wanted to be really rigorous about this sort of thing, we should
> change UInt32GetDatum to do something tangibly different from
> Int32GetDatum, like negate all the bits. Then if somebody picked the
> wrong macro it would actually fail. I'm not sure that's really the
> best place to spend our effort, though. The moral of this episode is
> that it's important to at least get the right width. Currently,
> getting the wrong signedness doesn't actually break anything.

Okay, understood. Thank you very much !

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com