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
diff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index 2d330ddb28..9999068134 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@ -13,7 +13,8 @@ OBJS = \
        rawpage.o
 
 EXTENSION = pageinspect
-DATA =  pageinspect--1.8--1.9.sql \
+DATA =  pageinspect--1.9--1.10.sql \
+       pageinspect--1.8--1.9.sql \
        pageinspect--1.7--1.8.sql pageinspect--1.6--1.7.sql \
        pageinspect--1.5.sql pageinspect--1.5--1.6.sql \
        pageinspect--1.4--1.5.sql pageinspect--1.3--1.4.sql \
diff --git a/contrib/pageinspect/expected/oldextversions.out 
b/contrib/pageinspect/expected/oldextversions.out
index 04dc7f8640..1bb3919504 100644
--- a/contrib/pageinspect/expected/oldextversions.out
+++ b/contrib/pageinspect/expected/oldextversions.out
@@ -36,5 +36,19 @@ SELECT * FROM bt_page_items('test1_a_idx', 1);
           1 | (0,1) |      16 | f     | f    | 01 00 00 00 00 00 00 01 | f    
| (0,1) | 
 (1 row)
 
+ALTER EXTENSION pageinspect UPDATE TO '1.9';
+\df page_header
+                                                                               
                                   List of functions
+ Schema |    Name     | Result data type |                                     
                                                    Argument data types         
                                                                                
| Type 
+--------+-------------+------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+------
+ public | page_header | record           | page bytea, OUT lsn pg_lsn, OUT 
checksum smallint, OUT flags smallint, OUT lower smallint, OUT upper smallint, 
OUT special smallint, OUT pagesize smallint, OUT version smallint, OUT 
prune_xid xid | func
+(1 row)
+
+SELECT pagesize, version FROM page_header(get_raw_page('test1', 0));
+ pagesize | version 
+----------+---------
+     8192 |       4
+(1 row)
+
 DROP TABLE test1;
 DROP EXTENSION pageinspect;
diff --git a/contrib/pageinspect/pageinspect--1.9--1.10.sql 
b/contrib/pageinspect/pageinspect--1.9--1.10.sql
new file mode 100644
index 0000000000..8dd9a27505
--- /dev/null
+++ b/contrib/pageinspect/pageinspect--1.9--1.10.sql
@@ -0,0 +1,21 @@
+/* contrib/pageinspect/pageinspect--1.9--1.10.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pageinspect UPDATE TO '1.10'" to load this file. 
\quit
+
+--
+-- page_header()
+--
+DROP FUNCTION page_header(IN page bytea);
+CREATE FUNCTION page_header(IN page bytea,
+    OUT lsn pg_lsn,
+    OUT checksum smallint,
+    OUT flags smallint,
+    OUT lower int,
+    OUT upper int,
+    OUT special int,
+    OUT pagesize int,
+    OUT version smallint,
+    OUT prune_xid xid)
+AS 'MODULE_PATHNAME', 'page_header'
+LANGUAGE C STRICT PARALLEL SAFE;
diff --git a/contrib/pageinspect/pageinspect.control 
b/contrib/pageinspect/pageinspect.control
index bd716769a1..7cdf37913d 100644
--- a/contrib/pageinspect/pageinspect.control
+++ b/contrib/pageinspect/pageinspect.control
@@ -1,5 +1,5 @@
 # pageinspect extension
 comment = 'inspect the contents of database pages at a low level'
-default_version = '1.9'
+default_version = '1.10'
 module_pathname = '$libdir/pageinspect'
 relocatable = true
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index e9fee73bc4..fdfb93ec1a 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -296,10 +296,59 @@ page_header(PG_FUNCTION_ARGS)
                values[0] = LSNGetDatum(lsn);
        values[1] = UInt16GetDatum(page->pd_checksum);
        values[2] = UInt16GetDatum(page->pd_flags);
-       values[3] = UInt16GetDatum(page->pd_lower);
-       values[4] = UInt16GetDatum(page->pd_upper);
-       values[5] = UInt16GetDatum(page->pd_special);
-       values[6] = UInt16GetDatum(PageGetPageSize(page));
+
+       switch (TupleDescAttr(tupdesc, 3)->atttypid)
+       {
+               case INT2OID:
+                       values[3] = UInt16GetDatum(page->pd_lower);
+                       break;
+               case INT4OID:
+                       values[3] = Int32GetDatum(page->pd_lower);
+                       break;
+               default:
+                       elog(ERROR, "incorrect output types");
+                       break;
+       }
+
+       switch (TupleDescAttr(tupdesc, 4)->atttypid)
+       {
+               case INT2OID:
+                       values[4] = UInt16GetDatum(page->pd_upper);
+                       break;
+               case INT4OID:
+                       values[4] = Int32GetDatum(page->pd_upper);
+                       break;
+               default:
+                       elog(ERROR, "incorrect output types");
+                       break;
+       }
+
+       switch (TupleDescAttr(tupdesc, 5)->atttypid)
+       {
+               case INT2OID:
+                       values[5] = UInt16GetDatum(page->pd_special);
+                       break;
+               case INT4OID:
+                       values[5] = Int32GetDatum(page->pd_special);
+                       break;
+               default:
+                       elog(ERROR, "incorrect output types");
+                       break;
+       }
+
+       switch (TupleDescAttr(tupdesc, 6)->atttypid)
+       {
+               case INT2OID:
+                       values[6] = UInt16GetDatum(PageGetPageSize(page));
+                       break;
+               case INT4OID:
+                       values[6] = Int32GetDatum(PageGetPageSize(page));
+                       break;
+               default:
+                       elog(ERROR, "incorrect output types");
+                       break;
+       }
+
        values[7] = UInt16GetDatum(PageGetPageLayoutVersion(page));
        values[8] = TransactionIdGetDatum(page->pd_prune_xid);
 
diff --git a/contrib/pageinspect/sql/oldextversions.sql 
b/contrib/pageinspect/sql/oldextversions.sql
index 78e08f40e8..b400e7ceeb 100644
--- a/contrib/pageinspect/sql/oldextversions.sql
+++ b/contrib/pageinspect/sql/oldextversions.sql
@@ -16,5 +16,9 @@ SELECT page_checksum(get_raw_page('test1', 0), 0) IS NOT NULL 
AS silly_checksum_
 SELECT * FROM bt_page_stats('test1_a_idx', 1);
 SELECT * FROM bt_page_items('test1_a_idx', 1);
 
+ALTER EXTENSION pageinspect UPDATE TO '1.9';
+\df page_header
+SELECT pagesize, version FROM page_header(get_raw_page('test1', 0));
+
 DROP TABLE test1;
 DROP EXTENSION pageinspect;

Reply via email to