Re: bugfix: when the blocksize is 32k, the function page_header of pageinspect returns negative numbers.

From: Quan Zongliang <quanzongliang(at)yeah(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: bugfix: when the blocksize is 32k, the function page_header of pageinspect returns negative numbers.
Date: 2021-07-09 03:11:46
Message-ID: 7f7646c0-9660-34f9-56b4-120c9eb49cbc@yeah.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021/7/9 9:50 上午, Michael Paquier wrote:
> On Fri, Jul 09, 2021 at 09:26:37AM +0800, Quan Zongliang wrote:
>> new patch attached
>
> That's mostly fine at quick glance. Here are some comments.
>
> Please add pageinspect--1.9--1.10.sql within the patch. Using git,
> you can do that with a simple "git add". With the current shape of
> the patch, one has to manually copy pageinspect--1.9--1.10.sql into
> contrib/pageinspect/ to be able to test it.
>
> +SELECT lower, upper, special, pagesize, version FROM
> page_header(get_raw_page('test1', 0));
> + lower | upper | special | pagesize | version
> +-------+-------+---------+----------+---------
> + 28 | 8152 | 8192 | 8192 | 4
> +(1 row)
> I would not test all the fields, just pagesize and version perhaps?
>
> + if (TupleDescAttr(tupdesc, 3)->atttypid == INT2OID)
> + values[3] = UInt16GetDatum(page->pd_lower);
> + else
> + values[3] = Int32GetDatum(page->pd_lower);
> Let's make the style more consistent with brinfuncs.c, by grouping all
> the fields together in a switch/case, like that:
> switch ((TupleDescAttr(tupdesc, 3)->atttypid)):
> {
> case INT2OID:
> /* fill in values with UInt16GetDatum() */
> break;
> case INT4OID:
> /* fill in values with Int32GetDatum() */
> break;
> default:
> elog(ERROR, "blah");
> }
>
> +ALTER EXTENSION pageinspect UPDATE TO '1.10';
> +\df page_header
> +SELECT lower, upper, special, pagesize, version FROM page_header(get_raw_page('test1', 0));
> No need for this test as page.sql already stresses page_header() for
> the latest version. Perhaps we could just UPDATE TO '1.9' before
> running your query to show that it is the last version of pageinspect
> where the older page_header() appeared.
>
> The documentation of pageinspect does not require an update, as far as
> I can see. So we are good there.
> --
> Michael
>

Thanks for the comments.
Done

Attachment Content-Type Size
pageinspect.patch text/plain 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message houzj.fnst@fujitsu.com 2021-07-09 03:28:30 RE: Added schema level support for publication.
Previous Message Michael Paquier 2021-07-09 02:59:43 Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE