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

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: pageinspect / add page_checksum and bt_page_items(bytea)
Date: 2017-03-29 15:08:41
Message-ID: 8060e483-abe1-9f26-6d47-9655341d55db@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
pageinspect-btree-bytea-v4.patch text/x-patch 0 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2017-03-29 15:13:35 Re: Logical decoding on standby
Previous Message Robert Haas 2017-03-29 15:07:18 Re: [COMMITTERS] pgsql: Improve access to parallel query from procedural languages.