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

2017-02-05 Thread Ashutosh Sharma
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

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

2017-02-05 Thread Robert Haas
On Fri, Feb 3, 2017 at 9:14 PM, Ashutosh Sharma wrote: > On Fri, Feb 3, 2017 at 8:29 PM, Robert Haas wrote: >> On Fri, Feb 3, 2017 at 7:41 AM, Ashutosh Sharma >> wrote: >>> I think UInt32GetDatum(metad->hashm_procid) looks

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

2017-02-03 Thread Ashutosh Sharma
On Fri, Feb 3, 2017 at 8:29 PM, Robert Haas wrote: > On Fri, Feb 3, 2017 at 7:41 AM, Ashutosh Sharma wrote: >> I think UInt32GetDatum(metad->hashm_procid) looks fine, the reason >> being 'hashm_procid' is defined as 32-bit unsigned integer but then

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

2017-02-03 Thread Robert Haas
On Fri, Feb 3, 2017 at 12:04 PM, Tom Lane wrote: > One thing to think about is what will happen if someday we want to use > 64-bit hash codes (a day I think is not that far away). It sounds like > you've already chosen bigint for any output field that represents a > hash code

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

2017-02-03 Thread Tom Lane
Robert Haas writes: > So based on that theory, here's a patch. > ... > In short, this patch makes hashfuncs.c consistent about (1) using the > next wider signed type to report unsigned values and (2) using the > GetDatum macro that matches the SQL return type in width and >

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

2017-02-03 Thread Tom Lane
Robert Haas writes: > Hopefully that will turn the buildfarm green again, but we'll see. It won't make the unhappy 64-bit machines happy, but I just pushed a change that should deal with those problems. With a little luck we're over the hump now.

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

2017-02-03 Thread Robert Haas
On Fri, Feb 3, 2017 at 10:54 AM, Robert Haas wrote: >> What needs to be resolved to decide if any of this is actually sane is to >> figure out which of these values need to be int64 on the SQL side because >> (a) they could practically exceed the range of signed int32 and

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

2017-02-03 Thread Robert Haas
On Fri, Feb 3, 2017 at 7:41 AM, Ashutosh Sharma wrote: >> I think for now selecting named fields is sufficient. > > +1. Attached is the patch that has this changes. Thanks for the patch, but you only handled one of the two cases Tom reported upthread. Added a fix for the

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

2017-02-03 Thread Robert Haas
On Fri, Feb 3, 2017 at 10:28 AM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Feb 2, 2017 at 11:16 PM, Tom Lane wrote: >>> I just made the C code agree with what the SQL declarations for the >>> functions say. > >> Doesn't look

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

2017-02-03 Thread Robert Haas
On Fri, Feb 3, 2017 at 7:41 AM, Ashutosh Sharma wrote: > 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