On Tue, Nov 20, 2012 at 4:44 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Hello,
>
>> Agreed. Attached patch introduces the pgstatginindex() which now reports
>> GIN version number, number of pages in the pending list and number of
>> tuples in the pending list, as information about a GIN index.
>
> It seems fine on the whole, and I have some suggestions.
Thanks for the review!
> 1. This patch applies current git head cleanly, but installation
> ends up with failure because of the lack of
> pgstattuple--1.0--1.1.sql which added in Makefile.
Added pgstattuple--1.0--1.1.sql.
> 2. I feel somewhat uneasy with size for palloc's (it's too long),
> and BuildTupleFromCString used instead of heap_from_tuple.. But
> it would lead additional modification for existent simillars.
>
> You can leave that if you prefer to keep this patch smaller,
> but it looks to me more preferable to construct the result
> tuple not via c-strings in some aspects. (*1)
OK. I changed the code as you suggested.
Updated version of the patch attached.
>
> 3. pgstatginidex shows only version, pending_pages, and
> pendingtuples. Why don't you show the other properties such as
> entry pages, data pages, entries, and total pages as
> pgstatindex does?
I didn't expose those because they are accurate as of last VACUUM.
But if you think they are useful, I don't object to expose them.
Regards,
--
Fujii Masao
