Re: pageinspect: Hash index support - Mailing list pgsql-hackers

Hi,

I am getting a fatal error along with some warnings when trying to
apply the v3 patch using "git apply" command.

[ashu@localhost postgresql]$ git apply
0001-pageinspect-Hash-index-support_v3.patch
0001-pageinspect-Hash-index-support_v3.patch:29: trailing whitespace.         brinfuncs.o ginfuncs.o hashfuncs.o
$(WIN32RES)
0001-pageinspect-Hash-index-support_v3.patch:36: trailing whitespace.
DATA = pageinspect--1.6.sql pageinspect--1.5--1.6.sql \
0001-pageinspect-Hash-index-support_v3.patch:37: trailing whitespace.   pageinspect--1.4--1.5.sql
pageinspect--1.3--1.4.sql\
 
0001-pageinspect-Hash-index-support_v3.patch:38: trailing whitespace.   pageinspect--1.2--1.3.sql
pageinspect--1.1--1.2.sql\
 
0001-pageinspect-Hash-index-support_v3.patch:39: trailing whitespace.   pageinspect--1.0--1.1.sql
pageinspect--unpackaged--1.0.sql
fatal: git apply: bad git-diff - expected /dev/null on line 46

Also, i think the USAGE for hash_metap() is refering to
hash_metap_bytea(). Please correct it. I have just started reviewing
the patch, will keep on posting my comments upon further review.
Thanks.

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



On Tue, Sep 20, 2016 at 10:15 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
> On Tue, Sep 20, 2016 at 5:40 AM, Jesper Pedersen
> <jesper.pedersen@redhat.com> wrote:
>>
>> On 09/20/2016 03:19 AM, Michael Paquier wrote:
>>>
>>> You did not get right the comments from Alvaro upthread. The following
>>> functions are added with this patch:
>>>  function hash_metap(text)
>>>  function hash_metap_bytea(bytea)
>>>  function hash_page_items(text,integer)
>>>  function hash_page_items_bytea(bytea)
>>>  function hash_page_stats(text,integer)
>>>  function hash_page_stats_bytea(bytea,integer)
>>>
>>> Now the following set of functions would be sufficient:
>>> function hash_metapage_info(bytea)
>>> function hash_page_items(bytea)
>>> function hash_page_stats(bytea)
>>> The last time pageinspect has been updated, when BRIN functions have
>>> been added, it has been discussed to just use (bytea) as an argument
>>> interface and just rely on get_raw_page() to get the pages wanted, so
>>> I think that we had better stick with that and keep things simple.
>>>
>>
>> Yes, I know, Alvaro and you voted for the bytea methods, and Jeff asked
>> for both.
>>
>> Attached is v3 with only the bytea based methods.
>>
>> Alvaro, Michael and Jeff - Thanks for the review !
>
>
>
> Is the 2nd "1" in this call needed?
>
> SELECT * FROM hash_page_stats(get_raw_page('mytab_index', 1), 1)
>
> As far as I can tell, that argument is only used to stuff into the output
> field "blkno", it is not used to instruct the interpretation of the raw page
> itself.  It doesn't seem worthwhile to have the parameter that only echos
> back to the user what the user already put in (twice).  The only existing
> funtions which take the blkno argument are those that don't use the
> get_raw_page style.
>
> Also, should we document what the single letter values mean in the
> hash_page_stats.type column?  It is not obvious that 'i' means bitmap, for
> example.
>
> Cheers,
>
> Jeff