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

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.

regards

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