Re: pageinspect: Hash index support

Hi,

I got some 'trailing whitespace' error (shown below) when git applying
v7 patch attached upthread. I have corrected the same and also ran
pgindent on a new file 'hashfuncs.c' added as a part of this project.
Attached is the updated v8 patch.

0001-Add-support-for-hash-index-in-pageinspect-contrib-mo_v7.patch:28:
trailing whitespace.
           brinfuncs.o ginfuncs.o hashfuncs.o $(WIN32RES)
0001-Add-support-for-hash-index-in-pageinspect-contrib-mo_v7.patch:35:
trailing whitespace.
DATA = pageinspect--1.5.sql pageinspect--1.5--1.6.sql \
0001-Add-support-for-hash-index-in-pageinspect-contrib-mo_v7.patch:36:
trailing whitespace.
    pageinspect--1.4--1.5.sql pageinspect--1.3--1.4.sql \
0001-Add-support-for-hash-index-in-pageinspect-contrib-mo_v7.patch:37:
trailing whitespace.
    pageinspect--1.2--1.3.sql pageinspect--1.1--1.2.sql \
0001-Add-support-for-hash-index-in-pageinspect-contrib-mo_v7.patch:38:
trailing whitespace.
    pageinspect--1.0--1.1.sql pageinspect--unpackaged--1.0.sql


pgindent:
-------------
./src/tools/pgindent/pgindent contrib/pageinspect/hashfuncs.c

On Wed, Jan 18, 2017 at 9:15 PM, Jesper Pedersen
<jesper.pedersen@redhat.com> wrote:
> Hi,
>
>
> On 01/18/2017 04:54 AM, Ashutosh Sharma wrote:
>>>
>>> Is there a reason for keeping the input arguments for
>>> hash_bitmap_info() different from hash_page_items()?
>>>
>>
>> Yes, there are two reasons behind it.
>>
>> Firstly, we need metapage to identify the bitmap page that holds the
>> information about the overflow page passed as an input to this
>> function.
>>
>> Secondly, we will have to input overflow block number as an input to
>> this function so as to determine the overflow bit number which can be
>> used further to identify the bitmap page.
>>
>> +   /* Read the metapage so we can determine which bitmap page to use */
>> +   metabuf = _hash_getbuf(indexRel, HASH_METAPAGE, HASH_READ,
>> LH_META_PAGE);
>> +   metap = HashPageGetMeta(BufferGetPage(metabuf));
>> +
>> +   /* Identify overflow bit number */
>> +   ovflbitno = _hash_ovflblkno_to_bitno(metap, ovflblkno);
>> +
>> +   bitmappage = ovflbitno >> BMPG_SHIFT(metap);
>> +   bitmapbit = ovflbitno & BMPG_MASK(metap);
>> +
>> +   if (bitmappage >= metap->hashm_nmaps)
>> +       ereport(ERROR,
>> +               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>> +                errmsg("invalid overflow bit number %u", ovflbitno)));
>> +
>> +   bitmapblkno = metap->hashm_mapp[bitmappage];
>>
>>
>
> As discussed off-list (along with my patch that included some of these
> fixes), it would require calculation from the user to be able to provide the
> necessary pages through get_raw_page().
>
> Other ideas on how to improve this are most welcome.
>
>> Apart from above comments, the attached patch also handles all the
>> comments from Mithun.
>>
>
> Please, include a version number for your patch files in the future.
>
> Fixed in this version:
>
> * verify_hash_page: Display magic in hex, like hash_metapage_info
> * Update header for hash_page_type
>
> Moving the patch back to 'Needs Review'.
>
> Best regards,
>  Jesper
>