pageinspect: Change block number arguments to bigint

Block numbers are 32-bit unsigned integers.  Therefore, the smallest
SQL integer type that they can fit in is bigint.  However, in the
pageinspect module, most input and output parameters dealing with
block numbers were declared as int.  The behavior with block numbers
larger than a signed 32-bit integer was therefore dubious.  Change
these arguments to type bigint and add some more explicit error
checking on the block range.

(Other contrib modules appear to do this correctly already.)

Since we are changing argument types of existing functions, in order
to not misbehave if the binary is updated before the extension is
updated, we need to create new C symbols for the entry points, similar
to how it's done in other extensions as well.

Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/d8f6bdd536df403b9b33816e9f7e0b9d@G08CNEXMBPEKD05.g08.fujitsu.local

Author: petere

contrib/pageinspect/Makefile

@@ -21,7 +21,7 @@ DATA =  pageinspect--1.8--1.9.sql \
 	pageinspect--1.0--1.1.sql
 PGFILEDESC = "pageinspect - functions to inspect contents of database pages"
 
-REGRESS = page btree brin gin gist hash checksum
+REGRESS = page btree brin gin gist hash checksum oldextversions
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config

contrib/pageinspect/brinfuncs.c

@@ -252,7 +252,18 @@ brin_page_items(PG_FUNCTION_ARGS)
 			int			att = attno - 1;
 
 			values[0] = UInt16GetDatum(offset);
-			values[1] = UInt32GetDatum(dtup->bt_blkno);
+			switch (TupleDescAttr(tupdesc, 1)->atttypid)
+			{
+				case INT8OID:
+					values[1] = Int64GetDatum((int64) dtup->bt_blkno);
+					break;
+				case INT4OID:
+					/* support for old extension version */
+					values[1] = UInt32GetDatum(dtup->bt_blkno);
+					break;
+				default:
+					elog(ERROR, "incorrect output types");
+			}
 			values[2] = UInt16GetDatum(attno);
 			values[3] = BoolGetDatum(dtup->bt_columns[att].bv_allnulls);
 			values[4] = BoolGetDatum(dtup->bt_columns[att].bv_hasnulls);

contrib/pageinspect/btreefuncs.c

@@ -41,8 +41,10 @@
 #include "utils/varlena.h"
 
 PG_FUNCTION_INFO_V1(bt_metap);
+PG_FUNCTION_INFO_V1(bt_page_items_1_9);
 PG_FUNCTION_INFO_V1(bt_page_items);
 PG_FUNCTION_INFO_V1(bt_page_items_bytea);
+PG_FUNCTION_INFO_V1(bt_page_stats_1_9);
 PG_FUNCTION_INFO_V1(bt_page_stats);
 
 #define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
@@ -160,11 +162,11 @@ GetBTPageStatistics(BlockNumber blkno, Buffer buffer, BTPageStat *stat)
  * Usage: SELECT * FROM bt_page_stats('t1_pkey', 1);
  * -----------------------------------------------
  */
-Datum
-bt_page_stats(PG_FUNCTION_ARGS)
+static Datum
+bt_page_stats_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version)
 {
 	text	   *relname = PG_GETARG_TEXT_PP(0);
-	uint32		blkno = PG_GETARG_UINT32(1);
+	int64		blkno = (ext_version == PAGEINSPECT_V1_8 ? PG_GETARG_UINT32(1) : PG_GETARG_INT64(1));
 	Buffer		buffer;
 	Relation	rel;
 	RangeVar   *relrv;
@@ -197,8 +199,15 @@ bt_page_stats(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot access temporary tables of other sessions")));
 
+	if (blkno < 0 || blkno > MaxBlockNumber)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("invalid block number")));
+
 	if (blkno == 0)
-		elog(ERROR, "block 0 is a meta page");
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("block 0 is a meta page")));
 
 	CHECK_RELATION_BLOCK_RANGE(rel, blkno);
 
@@ -219,16 +228,16 @@ bt_page_stats(PG_FUNCTION_ARGS)
 		elog(ERROR, "return type must be a row type");
 
 	j = 0;
-	values[j++] = psprintf("%d", stat.blkno);
+	values[j++] = psprintf("%u", stat.blkno);
 	values[j++] = psprintf("%c", stat.type);
-	values[j++] = psprintf("%d", stat.live_items);
-	values[j++] = psprintf("%d", stat.dead_items);
-	values[j++] = psprintf("%d", stat.avg_item_size);
-	values[j++] = psprintf("%d", stat.page_size);
-	values[j++] = psprintf("%d", stat.free_size);
-	values[j++] = psprintf("%d", stat.btpo_prev);
-	values[j++] = psprintf("%d", stat.btpo_next);
-	values[j++] = psprintf("%d", (stat.type == 'd') ? stat.btpo.xact : stat.btpo.level);
+	values[j++] = psprintf("%u", stat.live_items);
+	values[j++] = psprintf("%u", stat.dead_items);
+	values[j++] = psprintf("%u", stat.avg_item_size);
+	values[j++] = psprintf("%u", stat.page_size);
+	values[j++] = psprintf("%u", stat.free_size);
+	values[j++] = psprintf("%u", stat.btpo_prev);
+	values[j++] = psprintf("%u", stat.btpo_next);
+	values[j++] = psprintf("%u", (stat.type == 'd') ? stat.btpo.xact : stat.btpo.level);
 	values[j++] = psprintf("%d", stat.btpo_flags);
 
 	tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
@@ -239,6 +248,19 @@ bt_page_stats(PG_FUNCTION_ARGS)
 	PG_RETURN_DATUM(result);
 }
 
+Datum
+bt_page_stats_1_9(PG_FUNCTION_ARGS)
+{
+	return bt_page_stats_internal(fcinfo, PAGEINSPECT_V1_9);
+}
+
+/* entry point for old extension version */
+Datum
+bt_page_stats(PG_FUNCTION_ARGS)
+{
+	return bt_page_stats_internal(fcinfo, PAGEINSPECT_V1_8);
+}
+
 
 /*
  * cross-call data structure for SRF
@@ -405,11 +427,11 @@ bt_page_print_tuples(struct user_args *uargs)
  * Usage: SELECT * FROM bt_page_items('t1_pkey', 1);
  *-------------------------------------------------------
  */
-Datum
-bt_page_items(PG_FUNCTION_ARGS)
+static Datum
+bt_page_items_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version)
 {
 	text	   *relname = PG_GETARG_TEXT_PP(0);
-	uint32		blkno = PG_GETARG_UINT32(1);
+	int64		blkno = (ext_version == PAGEINSPECT_V1_8 ? PG_GETARG_UINT32(1) : PG_GETARG_INT64(1));
 	Datum		result;
 	FuncCallContext *fctx;
 	MemoryContext mctx;
@@ -447,8 +469,15 @@ bt_page_items(PG_FUNCTION_ARGS)
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("cannot access temporary tables of other sessions")));
 
+		if (blkno < 0 || blkno > MaxBlockNumber)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("invalid block number")));
+
 		if (blkno == 0)
-			elog(ERROR, "block 0 is a meta page");
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("block 0 is a meta page")));
 
 		CHECK_RELATION_BLOCK_RANGE(rel, blkno);
 
@@ -506,6 +535,19 @@ bt_page_items(PG_FUNCTION_ARGS)
 	SRF_RETURN_DONE(fctx);
 }
 
+Datum
+bt_page_items_1_9(PG_FUNCTION_ARGS)
+{
+	return bt_page_items_internal(fcinfo, PAGEINSPECT_V1_9);
+}
+
+/* entry point for old extension version */
+Datum
+bt_page_items(PG_FUNCTION_ARGS)
+{
+	return bt_page_items_internal(fcinfo, PAGEINSPECT_V1_8);
+}
+
 /*-------------------------------------------------------
  * bt_page_items_bytea()
  *

contrib/pageinspect/expected/btree.out

@@ -14,6 +14,8 @@ oldest_xact             | 0
 last_cleanup_num_tuples | -1
 allequalimage           | t
 
+SELECT * FROM bt_page_stats('test1_a_idx', -1);
+ERROR:  invalid block number
 SELECT * FROM bt_page_stats('test1_a_idx', 0);
 ERROR:  block 0 is a meta page
 SELECT * FROM bt_page_stats('test1_a_idx', 1);
@@ -32,6 +34,8 @@ btpo_flags    | 3
 
 SELECT * FROM bt_page_stats('test1_a_idx', 2);
 ERROR:  block number out of range
+SELECT * FROM bt_page_items('test1_a_idx', -1);
+ERROR:  invalid block number
 SELECT * FROM bt_page_items('test1_a_idx', 0);
 ERROR:  block 0 is a meta page
 SELECT * FROM bt_page_items('test1_a_idx', 1);
@@ -48,6 +52,8 @@ tids       |
 
 SELECT * FROM bt_page_items('test1_a_idx', 2);
 ERROR:  block number out of range
+SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', -1));
+ERROR:  invalid block number
 SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 0));
 ERROR:  block is a meta page
 SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 1));

contrib/pageinspect/expected/gin.out

@@ -35,3 +35,4 @@ FROM gin_leafpage_items(get_raw_page('test1_y_idx',
 -[ RECORD 1 ]
 ?column? | t
 
+DROP TABLE test1;

contrib/pageinspect/expected/hash.out

@@ -28,6 +28,8 @@ hash_page_type | bitmap
 
 SELECT hash_page_type(get_raw_page('test_hash_a_idx', 6));
 ERROR:  block number 6 is out of range for relation "test_hash_a_idx"
+SELECT * FROM hash_bitmap_info('test_hash_a_idx', -1);
+ERROR:  invalid block number
 SELECT * FROM hash_bitmap_info('test_hash_a_idx', 0);
 ERROR:  invalid overflow block number 0
 SELECT * FROM hash_bitmap_info('test_hash_a_idx', 1);
@@ -40,6 +42,8 @@ SELECT * FROM hash_bitmap_info('test_hash_a_idx', 4);
 ERROR:  invalid overflow block number 4
 SELECT * FROM hash_bitmap_info('test_hash_a_idx', 5);
 ERROR:  invalid overflow block number 5
+SELECT * FROM hash_bitmap_info('test_hash_a_idx', 6);
+ERROR:  block number 6 is out of range for relation "test_hash_a_idx"
 SELECT magic, version, ntuples, bsize, bmsize, bmshift, maxbucket, highmask,
 lowmask, ovflpoint, firstfree, nmaps, procid, spares, mapp FROM
 hash_metapage_info(get_raw_page('test_hash_a_idx', 0));

contrib/pageinspect/expected/oldextversions.out

@@ -0,0 +1,40 @@
+-- test old extension version entry points
+DROP EXTENSION pageinspect;
+CREATE EXTENSION pageinspect VERSION '1.8';
+CREATE TABLE test1 (a int8, b text);
+INSERT INTO test1 VALUES (72057594037927937, 'text');
+CREATE INDEX test1_a_idx ON test1 USING btree (a);
+-- from page.sql
+SELECT octet_length(get_raw_page('test1', 0)) AS main_0;
+ main_0 
+--------
+   8192
+(1 row)
+
+SELECT octet_length(get_raw_page('test1', 'main', 0)) AS main_0;
+ main_0 
+--------
+   8192
+(1 row)
+
+SELECT page_checksum(get_raw_page('test1', 0), 0) IS NOT NULL AS silly_checksum_test;
+ silly_checksum_test 
+---------------------
+ t
+(1 row)
+
+-- from btree.sql
+SELECT * FROM bt_page_stats('test1_a_idx', 1);
+ blkno | type | live_items | dead_items | avg_item_size | page_size | free_size | btpo_prev | btpo_next | btpo | btpo_flags 
+-------+------+------------+------------+---------------+-----------+-----------+-----------+-----------+------+------------
+     1 | l    |          1 |          0 |            16 |      8192 |      8128 |         0 |         0 |    0 |          3
+(1 row)
+
+SELECT * FROM bt_page_items('test1_a_idx', 1);
+ itemoffset | ctid  | itemlen | nulls | vars |          data           | dead | htid  | tids 
+------------+-------+---------+-------+------+-------------------------+------+-------+------
+          1 | (0,1) |      16 | f     | f    | 01 00 00 00 00 00 00 01 | f    | (0,1) | 
+(1 row)
+
+DROP TABLE test1;
+DROP EXTENSION pageinspect;

contrib/pageinspect/expected/page.out

@@ -32,6 +32,8 @@ SELECT octet_length(get_raw_page('test1', 'vm', 0)) AS vm_0;
 
 SELECT octet_length(get_raw_page('test1', 'vm', 1)) AS vm_1;
 ERROR:  block number 1 is out of range for relation "test1"
+SELECT octet_length(get_raw_page('test1', 'main', -1));
+ERROR:  invalid block number
 SELECT octet_length(get_raw_page('xxx', 'main', 0));
 ERROR:  relation "xxx" does not exist
 SELECT octet_length(get_raw_page('test1', 'xxx', 0));
@@ -55,6 +57,8 @@ SELECT page_checksum(get_raw_page('test1', 0), 0) IS NOT NULL AS silly_checksum_
  t
 (1 row)
 
+SELECT page_checksum(get_raw_page('test1', 0), -1);
+ERROR:  invalid block number
 SELECT tuple_data_split('test1'::regclass, t_data, t_infomask, t_infomask2, t_bits)
     FROM heap_page_items(get_raw_page('test1', 0));
        tuple_data_split        

contrib/pageinspect/hashfuncs.c

@@ -390,7 +390,7 @@ Datum
 hash_bitmap_info(PG_FUNCTION_ARGS)
 {
 	Oid			indexRelid = PG_GETARG_OID(0);
-	uint64		ovflblkno = PG_GETARG_INT64(1);
+	int64		ovflblkno = PG_GETARG_INT64(1);
 	HashMetaPage metap;
 	Buffer		metabuf,
 				mapbuf;
@@ -425,11 +425,16 @@ hash_bitmap_info(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot access temporary tables of other sessions")));
 
+	if (ovflblkno < 0 || ovflblkno > MaxBlockNumber)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("invalid block number")));
+
 	if (ovflblkno >= RelationGetNumberOfBlocks(indexRel))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("block number " UINT64_FORMAT " is out of range for relation \"%s\"",
-						ovflblkno, RelationGetRelationName(indexRel))));
+				 errmsg("block number %lld is out of range for relation \"%s\"",
+						(long long int) ovflblkno, RelationGetRelationName(indexRel))));
 
 	/* Read the metapage so we can determine which bitmap page to use */
 	metabuf = _hash_getbuf(indexRel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);

contrib/pageinspect/pageinspect--1.8--1.9.sql

@@ -39,3 +39,80 @@ CREATE FUNCTION gist_page_items(IN page bytea,
 RETURNS SETOF record
 AS 'MODULE_PATHNAME', 'gist_page_items'
 LANGUAGE C STRICT PARALLEL SAFE;
+
+--
+-- get_raw_page()
+--
+DROP FUNCTION get_raw_page(text, int4);
+CREATE FUNCTION get_raw_page(text, int8)
+RETURNS bytea
+AS 'MODULE_PATHNAME', 'get_raw_page_1_9'
+LANGUAGE C STRICT PARALLEL SAFE;
+
+DROP FUNCTION get_raw_page(text, text, int4);
+CREATE FUNCTION get_raw_page(text, text, int8)
+RETURNS bytea
+AS 'MODULE_PATHNAME', 'get_raw_page_fork_1_9'
+LANGUAGE C STRICT PARALLEL SAFE;
+
+--
+-- page_checksum()
+--
+DROP FUNCTION page_checksum(IN page bytea, IN blkno int4);
+CREATE FUNCTION page_checksum(IN page bytea, IN blkno int8)
+RETURNS smallint
+AS 'MODULE_PATHNAME', 'page_checksum_1_9'
+LANGUAGE C STRICT PARALLEL SAFE;
+
+--
+-- bt_page_stats()
+--
+DROP FUNCTION bt_page_stats(text, int4);
+CREATE FUNCTION bt_page_stats(IN relname text, IN blkno int8,
+    OUT blkno int8,
+    OUT type "char",
+    OUT live_items int4,
+    OUT dead_items int4,
+    OUT avg_item_size int4,
+    OUT page_size int4,
+    OUT free_size int4,
+    OUT btpo_prev int8,
+    OUT btpo_next int8,
+    OUT btpo int4,
+    OUT btpo_flags int4)
+AS 'MODULE_PATHNAME', 'bt_page_stats_1_9'
+LANGUAGE C STRICT PARALLEL SAFE;
+
+--
+-- bt_page_items()
+--
+DROP FUNCTION bt_page_items(text, int4);
+CREATE FUNCTION bt_page_items(IN relname text, IN blkno int8,
+    OUT itemoffset smallint,
+    OUT ctid tid,
+    OUT itemlen smallint,
+    OUT nulls bool,
+    OUT vars bool,
+    OUT data text,
+    OUT dead boolean,
+    OUT htid tid,
+    OUT tids tid[])
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'bt_page_items_1_9'
+LANGUAGE C STRICT PARALLEL SAFE;
+
+--
+-- brin_page_items()
+--
+DROP FUNCTION brin_page_items(IN page bytea, IN index_oid regclass);
+CREATE FUNCTION brin_page_items(IN page bytea, IN index_oid regclass,
+    OUT itemoffset int,
+    OUT blknum int8,
+    OUT attnum int,
+    OUT allnulls bool,
+    OUT hasnulls bool,
+    OUT placeholder bool,
+    OUT value text)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'brin_page_items'
+LANGUAGE C STRICT PARALLEL SAFE;