GIN pageinspect functions

Some time ago, when debugging a GIN bug, I wrote these pageinspect
functions to inspect GIN indexes. They were very useful; we should add them.

- Heikki

On Tue, Oct 7, 2014 at 9:03 PM, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
wrote:

> Some time ago, when debugging a GIN bug, I wrote these pageinspect
> functions to inspect GIN indexes. They were very useful; we should add them.
>

May be we can merge it with contrib/gevel, which we use many years for
development and debug purposes ? Have you seen it ?

>
> - Heikki
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

On 10/07/2014 08:36 PM, Oleg Bartunov wrote:
> On Tue, Oct 7, 2014 at 9:03 PM, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
> wrote:
>
>> Some time ago, when debugging a GIN bug, I wrote these pageinspect
>> functions to inspect GIN indexes. They were very useful; we should add them.
>>
>
> May be we can merge it with contrib/gevel, which we use many years for
> development and debug purposes ? Have you seen it ?

I remember downloading it many years ago, but that's all I remember.
Where's the latest version?

- Heikki

On Tue, Oct 7, 2014 at 9:56 PM, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
wrote:

> On 10/07/2014 08:36 PM, Oleg Bartunov wrote:
>
>> On Tue, Oct 7, 2014 at 9:03 PM, Heikki Linnakangas <
>> hlinnakangas(at)vmware(dot)com>
>> wrote:
>>
>> Some time ago, when debugging a GIN bug, I wrote these pageinspect
>>> functions to inspect GIN indexes. They were very useful; we should add
>>> them.
>>>
>>>
>> May be we can merge it with contrib/gevel, which we use many years for
>> development and debug purposes ? Have you seen it ?
>>
>
> I remember downloading it many years ago, but that's all I remember.
> Where's the latest version?
>

I believe it's there
http://www.sigaev.ru/git/gitweb.cgi?p=gevel.git;a=summary

> - Heikki
>

On Tue, Oct 7, 2014 at 10:33 PM, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
wrote:
>
> Some time ago, when debugging a GIN bug, I wrote these pageinspect
functions to inspect GIN indexes. They were very useful; we should add them.
>

I think these functions will be quite useful for debugging purpose
and we already have similar function's for other index (btree).

Few suggestions for patch:

1. Documentation seems to be missing, other API's exposed
via pageinspect are documented at:
http://www.postgresql.org/docs/devel/static/pageinspect.html

2.
+CREATE FUNCTION gin_metapage(IN page bytea,
+ OUT pending_head bigint,
+ OUT pending_tail bigint,
+
OUT tail_free_size int4,
+ OUT n_pending_pages bigint,
+ OUT n_pending_tuples bigint,
+ OUT
n_total_pages bigint,
+ OUT n_entry_pages bigint,
+ OUT n_data_pages bigint,
+ OUT n_entries bigint,
+
OUT version int4)
+AS 'MODULE_PATHNAME', 'gin_metapage'
+LANGUAGE C STRICT;

a. Isn't it better to name the function as gin_metap(..) similar to
existing function bt_metap(..)?
b. Can this function have a similar signature as bt_metap() which means
it should take input as relname?

3. Can gin_dataleafpage() API have similar name and signature as
API bt_page_items() exposed for btree?

4. Can we have any better name for gin_pageopaq (other API name's
in this module are self explanatory)?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Tue, Nov 4, 2014 at 7:26 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> I think these functions will be quite useful for debugging purpose
> and we already have similar function's for other index (btree).

This patch has bitrotted. I attach rebased revision, for the
convenience of others - V1.3 of pageinspect will now incorporate both
GIN stuff, and BRIN stuff. Seems like this patch was affected by the
recent problems with header includes - that's fixed.

Do you intend to fix this up?

+ /* TODO: array of decoded item pointers */
+ nulls[2] = true;

--
Peter Geoghegan

On Tue, Nov 18, 2014 at 2:01 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> Do you intend to fix this up?

BTW, how do you feel about the B-Tree check extension [1]? It's very
much related to pageinspect -- it's more or less a derivative. I don't
think I'm going to have time (or that there is sufficient review
bandwidth available) to get it into 9.5, but I should post a revision
soon, so it's at least something that's available for use by an
expert. I did some clean-up work on it that is unpublished. It'll
become a more generic extension - "amcheck", per Robert's suggestion.

One unpublished additional feature (that I have to fix a bug in) that
isn't included in [1] is the idea of checking invariants across B-Tree
pages. So, a scankey should indicate that the greatest (non-highkey)
item on a non-rightmost page comports with the page that it has a
right link to. Without race conditions.

I don't have that swapped into my head at the moment, and so I don't
have a good sense of how hard it'll be to fix the problem I found...

[1] http://www.postgresql.org/message-id/CAM3SWZRtV+xmRWLWq6c-x7czvwavFdwFi4St1zz4dDgFH4yN4g@mail.gmail.com
--
Peter Geoghegan

On Wed, Nov 19, 2014 at 7:01 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Tue, Nov 4, 2014 at 7:26 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> I think these functions will be quite useful for debugging purpose
>> and we already have similar function's for other index (btree).
>
> This patch has bitrotted. I attach rebased revision, for the
> convenience of others - V1.3 of pageinspect will now incorporate both
> GIN stuff, and BRIN stuff. Seems like this patch was affected by the
> recent problems with header includes - that's fixed.

Thanks for the updated version! That's a cool timing, I wanted to move
this patch forward these days... I think as well that it would be a
useful addition for pageinspect (and I was in need of them yesterday,
and soon today btw).

> Amit wrote:
> 1. Documentation seems to be missing, other API's exposed
> via pageinspect are documented at:
> http://www.postgresql.org/docs/devel/static/pageinspect.html
Done.

> 2.
> +CREATE FUNCTION gin_metapage(IN page bytea,
> + OUT pending_head bigint,
> + OUT pending_tail bigint,
> + OUT version int4)
> +AS 'MODULE_PATHNAME', 'gin_metapage'
> +LANGUAGE C STRICT;
> a. Isn't it better to name the function as gin_metap(..) similar to
> existing function bt_metap(..)?
I actually liked more gin_metapage_info, a name similar to the
newly-introduced brin indexes.

> b. Can this function have a similar signature as bt_metap() which means
> it should take input as relname?
That's mostly a matter of taste but I think we should definitely pass
a raw page to it as it is now. This has the advantage to add an extra
check if the page passed is really a meta page of not, something
useful for development.

> 3. Can gin_dataleafpage() API have similar name and signature as
> API bt_page_items() exposed for btree?
What about gin_leafpage_items then?

> 4. Can we have any better name for gin_pageopaq (other API name's
> in this module are self explanatory)?
gin_page_opaque_info? Because we get back information about the opaque
portion of the page. Feel free if you have any better idea.

Updated patch, with some more things improved and cleaned up (addition
of header of ginfuncs.c, addition of array of decoded item pointers
for compressed data leaf pages), is attached.

One last thing not only interesting for this patch: it may be good to
expose DatumGetItemPointer and ItemPointerGetDatum in for extensions
analyzing content of pages. I am not sure where though, a place like
utils/*.h may be useful. Thoughts?
Regards,
--
Michael

On 11/20/2014 05:52 AM, Michael Paquier wrote:
> On Wed, Nov 19, 2014 at 7:01 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> On Tue, Nov 4, 2014 at 7:26 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> 1. Documentation seems to be missing, other API's exposed
>> via pageinspect are documented at:
>> http://www.postgresql.org/docs/devel/static/pageinspect.html
> Done.
>
>> 2.
>> +CREATE FUNCTION gin_metapage(IN page bytea,
>> + OUT pending_head bigint,
>> + OUT pending_tail bigint,
>> + OUT version int4)
>> +AS 'MODULE_PATHNAME', 'gin_metapage'
>> +LANGUAGE C STRICT;
>> a. Isn't it better to name the function as gin_metap(..) similar to
>> existing function bt_metap(..)?
> I actually liked more gin_metapage_info, a name similar to the
> newly-introduced brin indexes.
>
>> b. Can this function have a similar signature as bt_metap() which means
>> it should take input as relname?
> That's mostly a matter of taste but I think we should definitely pass
> a raw page to it as it is now. This has the advantage to add an extra
> check if the page passed is really a meta page of not, something
> useful for development.
>
>> 3. Can gin_dataleafpage() API have similar name and signature as
>> API bt_page_items() exposed for btree?
> What about gin_leafpage_items then?

The signature of bt_page_items() isn't a good example to follow. It
existed before the get_raw_page() function, and the other functions that
are designed to work with that, was added. gin_leafpage_items() name
seems fine to me.

>> 4. Can we have any better name for gin_pageopaq (other API name's
>> in this module are self explanatory)?
> gin_page_opaque_info? Because we get back information about the opaque
> portion of the page. Feel free if you have any better idea.
>
> Updated patch, with some more things improved and cleaned up (addition
> of header of ginfuncs.c, addition of array of decoded item pointers
> for compressed data leaf pages), is attached.

This is why I love open source - I post something half-baked, and others
pop up and finish the work ;-). Committed with minor fixes, many thanks!

> One last thing not only interesting for this patch: it may be good to
> expose DatumGetItemPointer and ItemPointerGetDatum in for extensions
> analyzing content of pages. I am not sure where though, a place like
> utils/*.h may be useful. Thoughts?

Yeah, maybe. I'll leave that to the next patch that needs it, as long as
there's only one user of it, it doesn't seem worth it.

- Heikki

On Fri, Nov 21, 2014 at 2:04 AM, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com
> wrote:

> On 11/20/2014 05:52 AM, Michael Paquier wrote:
>
>> On Wed, Nov 19, 2014 at 7:01 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>>
>>> On Tue, Nov 4, 2014 at 7:26 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
>>> wrote:
>>> 1. Documentation seems to be missing, other API's exposed
>>> via pageinspect are documented at:
>>> http://www.postgresql.org/docs/devel/static/pageinspect.html
>>>
>> Done.
>>
>> 2.
>>> +CREATE FUNCTION gin_metapage(IN page bytea,
>>> + OUT pending_head bigint,
>>> + OUT pending_tail bigint,
>>> + OUT version int4)
>>> +AS 'MODULE_PATHNAME', 'gin_metapage'
>>> +LANGUAGE C STRICT;
>>> a. Isn't it better to name the function as gin_metap(..) similar to
>>> existing function bt_metap(..)?
>>>
>> I actually liked more gin_metapage_info, a name similar to the
>> newly-introduced brin indexes.
>>
>> b. Can this function have a similar signature as bt_metap() which means
>>> it should take input as relname?
>>>
>> That's mostly a matter of taste but I think we should definitely pass
>> a raw page to it as it is now. This has the advantage to add an extra
>> check if the page passed is really a meta page of not, something
>> useful for development.
>>
>> 3. Can gin_dataleafpage() API have similar name and signature as
>>> API bt_page_items() exposed for btree?
>>>
>> What about gin_leafpage_items then?
>>
>
> The signature of bt_page_items() isn't a good example to follow. It
> existed before the get_raw_page() function, and the other functions that
> are designed to work with that, was added. gin_leafpage_items() name seems
> fine to me.

When I call gin_leafpage_items on a {leaf} page, I get the ERROR:

ERROR: input page is not a compressed GIN data leaf page
DETAIL: Flags 0002, expected 0083

I'm don't know why it won't work on an uncompressed leaf page (or for that
matter, why my index pages are not compressed), but the docs should
probably note the restriction.

Cheers,

Jeff

On Mon, Aug 10, 2015 at 09:14:48AM -0700, Jeff Janes wrote:
> When I call gin_leafpage_items on a {leaf} page, I get the ERROR:
>
> ERROR:  input page is not a compressed GIN data leaf page
> DETAIL:  Flags 0002, expected 0083
>
> I'm don't know why it won't work on an uncompressed leaf page (or for that
> matter, why my index pages are not compressed), but the docs should probably
> note the restriction.

Yes, this patch is from nine years ago, but it is still an improvement,
and the GIN page still must be compressed, so patch applied to master:

opaq = GinPageGetOpaque(page);
if (opaq->flags != (GIN_DATA | GIN_LEAF | GIN_COMPRESSED))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("input page is not a compressed GIN data leaf page"),
errdetail("Flags %04X, expected %04X",
opaq->flags,
(GIN_DATA | GIN_LEAF | GIN_COMPRESSED))));

--
Bruce Momjian <bruce(at)momjian(dot)us> https://momjian.us
EDB https://enterprisedb.com

Only you can decide what is important to you.