Re: [PATCH] Pageinspect - add functions on GIN and GiST indexes from gevel

On Fri, Sep 8, 2017 at 3:38 AM, Tomas Vondra
<tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
> Hi,
>
> On 07/21/2017 03:40 PM, Alexander Korotkov wrote:
>> Hi, Alexey!
>>
>> On Fri, Jul 21, 2017 at 3:05 PM, Alexey Chernyshov
>> <a(dot)chernyshov(at)postgrespro(dot)ru <mailto:a(dot)chernyshov(at)postgrespro(dot)ru>> wrote:
>>
>> the following patch transfers functionality from gevel module
>> (http://www.sai.msu.su/~megera/wiki/Gevel
>> <http://www.sai.msu.su/~megera/wiki/Gevel>) which provides functions for
>> analyzing GIN and GiST indexes to pageinspect. Gevel was originally
>> designed by Oleg Bartunov, and Teodor Sigaev for developers of GiST and
>> GIN indexes.
>>
>>
>> It's very good that you've picked up this work! pageinspect is lacking
>> of this functionality.
>>
>> Functions added:
>> - gist_stat(text) - shows statistics on GiST Tree
>> - gist_tree(text) - shows GiST tree
>> - gist_tree(text, int4) - shows GiST tree up to MAXLEVEL
>> - gist_print(text) - prints objects stored in GiST tree
>> - spgist_stat(text) - shows statistics on SP-GiST
>> - spgist_print(text) - prints objects stored in index
>> - gin_value_count() - originally gin_stat(text) - prints estimated
>> counts
>> for index values
>> - gin_stats() - originally gin_statpage(text) - shows statistics
>> - gin_count_estimate(text, tsquery) - shows number of indexed rows
>> matched
>> query
>>
>> Tests also transferred, docs for new functions are added. I run pgindent
>> over the code, but the result is different from those I expected, so
>> I leave
>> pgindented one.
>> The patch is applicable to the commit
>> 866f4a7c210857aa342bf901558d170325094dde.
>>
>>
>> As we can see, gevel contains functions which analyze the whole index.
>> pageinspect is written in another manner: it gives you functionality to
>> analyze individual pages, tuples and so on.
>> Thus, we probably shouldn't try to move gevel functions to pageinspect
>> "as is". They should be rewritten in more granular manner as well as
>> other pageinspact functions are. Any other opinions?
>>
>
> I agree with Alexander that pageinspect is written in a very different
> way - as the extension name suggests, it's used to inspect pages. The
> proposed patch uses a very different approach, reading the whole index,
> not individual pages. Why should it be part of pageinspect?
>
> For example we have pgstattuple extension, which seems like a better
> match. Or just create a new extension - if the code is valuable, surely
> we can live one more extension instead of smuggling it in inside
> pageinspect.
>

Finally, i got some time to look into this patch and surprisingly I
didn't find any function returning information at page level instead
all the SQL functions are returning information at index level.
Therefore, i too feel that it should be either integrated with
pgstattuple which could a better match as Tomas also mentioned or may
be add a new contrib module itself. I think, adding a new contrib
module looks like a better option. The reason being, it doesn't simply
include the function for showing index level statistics (for e.g..
index size, no of rows, values..e.t.c) like pgstattuple does but,
also, displays information contained in a page for e.g. the object
stored in the page, it's tid e.t.c. So, more or less, I would say
that, it's the mixture of pageinspect and pgstattuple module and
therefore, i feel, adding it as a new extension would be a better
choice. Thought?

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