Re: [HACKERS] [PATCH] Pageinspect - add functions on GIN and GiSTindexes from gevel - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [HACKERS] [PATCH] Pageinspect - add functions on GIN and GiSTindexes from gevel
Date
Msg-id 2514c060-717e-6fb9-981c-bf1344174da5@2ndquadrant.com
Whole thread Raw
In response to Re: [HACKERS] [PATCH] Pageinspect - add functions on GIN and GiSTindexes from gevel  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Responses Re: [HACKERS] [PATCH] Pageinspect - add functions on GIN and GiSTindexes from gevel
List pgsql-hackers
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.chernyshov@postgrespro.ru <mailto:a.chernyshov@postgrespro.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.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



pgsql-hackers by date:

Previous
From: Magnus Hagander
Date:
Subject: Re: [HACKERS] GnuTLS support
Next
From: Thomas Munro
Date:
Subject: Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager