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-06 21:33:15 |
Message-ID: | 373c2db8-0c93-4962-3c4b-a17a562c5737@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 03/06/2017 10:13 PM, Peter Eisentraut wrote:
> On 3/3/17 09:03, Tomas Vondra wrote:
>> Attached is v2, fixing both issues.
>
> I wonder if
>
> + bytea *raw_page = PG_GETARG_BYTEA_P(0);
>
> + uargs->page = VARDATA(raw_page);
>
> is expected to work reliably, without copying the argument to a
> different memory context.
>
> I think it would be better not to maintain so much duplicate code
> between bt_page_items(text, int) and bt_page_items(bytea). How about
> just redefining bt_page_items(text, int) as an SQL-language function
> calling bt_page_items(get_raw_page($1, $2))?
>
Maybe. We can also probably share the code at the C level, so I'll look
into that.
>
> For page_checksum(), the checksums are internally unsigned, so maybe
> return type int on the SQL level, so that there is no confusion about
> the sign?
>
That ship already sailed, I'm afraid. We already have checksum in the
page_header() output, and it's defined as smallint there. So using int
here would be inconsistent.
A similar issue is that get_raw_page() uses int for block number, while
internally it's uint32. That often requires a fair amount of gymnastics
on large tables.
I'm not against changing either of those bits - using int for checksums
and bigint for block numbers, but I think it should be a separate patch.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-03-06 21:54:00 | Re: Parallel seq. plan is not coming against inheritance or partition table |
Previous Message | Kevin Grittner | 2017-03-06 21:24:43 | Re: WARNING: relcache reference leak: relation "p1" not closed |