On Mon, Sep 21, 2020 at 6:47 AM Hou, Zhijie <houzj.fnst@cn.fujitsu.com> wrote:
>
> In (/contrib/bloom/blutils.c:277), I found it use DatumGetInt32 to get UInt32 type.
> Is it more appropriate to use DatumGetUInt32 here?
>
Makes sense. +1 for the patch. I think with the existing code also we
don't have any problem. If we assume that the hash functions return
uint32, with DatumGetInt32() we are typecasting that uint32 result to
int32, we are assigning it to uint32 i.e. typecasting int32 back to
uint32. Eventually, I think, we will see the proper value in hashVal.
I did a small experiment to prove this [1].
uint32 hashVal;
hashVal = DatumGetInt32(FunctionCall1Coll(&state->hashFn[attno],
state->collations[attno], value));
It's good to run a few test cases/test suites(if they exist) that hit
this part of the code, just to ensure we don't break anything.
[1]
int main()
{
unsigned int u = 3 * 1024 * 1024 * 1024;
printf("%u\n", u);
int i = u;
printf("%d\n", i);
unsigned int u1 = i;
printf("%u\n", u1);
return 0;
}
Output of the above snippet:
3221225472
-1073741824
3221225472
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com