Re: PATCH: pageinspect / add page_checksum and bt_page_items(bytea)

On 03/24/2017 04:27 AM, Peter Eisentraut wrote:
> On 3/17/17 18:35, Tomas Vondra wrote:
>> On 03/17/2017 05:23 PM, Peter Eisentraut wrote:
>>> I'm struggling to find a good way to share code between
>>> bt_page_items(text, int4) and bt_page_items(bytea).
>>>
>>> If we do it via the SQL route, as I had suggested, it makes the
>>> extension non-relocatable, and it will also create a bit of a mess
>>> during upgrades.
>>>
>>> If doing it in C, it will be a bit tricky to pass the SRF context
>>> around. There is no "DirectFunctionCall within SRF context", AFAICT.
>>
>> Not sure what it has to do with DirectFunctionCall? You want to call the
>> bytea variant from the existing one? Wouldn't it be easier to simply
>> define a static function with the shared parts, and pass around the
>> fctx/fcinfo? Not quite pretty, but should work.
>
> Perhaps what was added in
> <http://git.postgresql.org/pg/commitdiff/29bf5016835a2c2c23786f7cda347716c083d95f>
> would actually work here.
>

I've tried to refactor the code using this, but the result was rather
ugly because (a) it really is quite tricky to pass around the contexts
and (b) the sanity checks are quite different for the two input types,
so mixing those parts (essentially the SRF_IS_FIRSTCALL bits) does not
make much sense IMHO.

The attached patch is the best I came up with - it essentially shares
just the tuple-forming part, which is exactly the same in both cases.

It also adds the P_ISMETA(opaque) check to the original function, which
seems like a useful defense against page written to a different place
(which is essentially the issue I was originally investigating).

regards

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