On 11/20/2014 05:52 AM, Michael Paquier wrote:
On Wed, Nov 19, 2014 at 7:01 AM, Peter Geoghegan <p...@heroku.com> wrote:
On Tue, Nov 4, 2014 at 7:26 AM, Amit Kapila <amit.kapil...@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
