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
signature.asc
Description: PGP signature