On Thu, 28 Jul 2022 at 00:36, Tom Lane <t...@sss.pgh.pa.us> wrote: > Hamid Akhtar <hamid.akh...@gmail.com> writes: > > That attached patch is based on the master branch. It makes the following > > changes to the pageinspect contrib module: > > - Updates bt_page_stats_internal function to accept 3 arguments instead > of > > 2. > > - The function now uses SRF macros to return a set rather than a single > > row. The function call now requires specifying column names. > > FWIW, I think you'd be way better off changing the function name, say > to bt_multi_page_stats(). Overloading the name this way is going to > lead to great confusion, e.g. somebody who fat-fingers the number of > output arguments in a JDBC call could see confusing results due to > invoking the wrong one of the two functions. Also, I'm not quite sure > what you mean by "The function call now requires specifying column > names", but it doesn't sound like an acceptable restriction from a > compatibility standpoint. If a different name dodges that issue then > it's clearly a better way. > > regards, tom lane >
Attached please find the latest version of the patch; pageinspect_btree_multipagestats_02.patch. It no longer modifies the existing bt_page_stats function. Instead, it introduces a new function bt_multi_page_stats as you had suggested. The function expects three arguments where the first argument is the index name followed by block number and number of blocks to be returned. Please ignore this statement. It was a typo. "The function call now requires specifying column names"
diff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile index 5c07365..31d7b1c 100644 --- a/contrib/pageinspect/Makefile +++ b/contrib/pageinspect/Makefile @@ -13,12 +13,12 @@ OBJS = \ rawpage.o EXTENSION = pageinspect -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 \ - pageinspect--1.2--1.3.sql pageinspect--1.1--1.2.sql \ - pageinspect--1.0--1.1.sql +DATA = pageinspect--1.10--1.11.sql 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 pageinspect--1.2--1.3.sql \ + pageinspect--1.1--1.2.sql pageinspect--1.0--1.1.sql PGFILEDESC = "pageinspect - functions to inspect contents of database pages" REGRESS = page btree brin gin gist hash checksum oldextversions diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index 62f2c1b..5dd216e 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -46,6 +46,7 @@ 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); +PG_FUNCTION_INFO_V1(bt_multi_page_stats); #define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX) #define IS_BTREE(r) ((r)->rd_rel->relam == BTREE_AM_OID) @@ -80,6 +81,26 @@ typedef struct BTPageStat BTCycleId btpo_cycleid; } BTPageStat; +/* + * cross-call data structure for SRF for page stats + */ +typedef struct ua_page_stats +{ + uint64 blkno; + uint64 blk_count; +} ua_page_stats; + +/* + * cross-call data structure for SRF for page items + */ +typedef struct ua_page_items +{ + Page page; + OffsetNumber offset; + bool leafpage; + bool rightmost; + TupleDesc tupd; +} ua_page_items; /* ------------------------------------------------- * GetBTPageStatistics() @@ -177,34 +198,15 @@ GetBTPageStatistics(BlockNumber blkno, Buffer buffer, BTPageStat *stat) } /* ----------------------------------------------- - * bt_page_stats() + * bt_index_block_validate() * - * Usage: SELECT * FROM bt_page_stats('t1_pkey', 1); + * Validate index type is btree and block number + * is within a valid block number range. * ----------------------------------------------- */ -static Datum -bt_page_stats_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version) +static void +bt_index_block_validate(Relation rel, int64 blkno) { - text *relname = PG_GETARG_TEXT_PP(0); - int64 blkno = (ext_version == PAGEINSPECT_V1_8 ? PG_GETARG_UINT32(1) : PG_GETARG_INT64(1)); - Buffer buffer; - Relation rel; - RangeVar *relrv; - Datum result; - HeapTuple tuple; - TupleDesc tupleDesc; - int j; - char *values[11]; - BTPageStat stat; - - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("must be superuser to use pageinspect functions"))); - - relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname)); - rel = relation_openrv(relrv, AccessShareLock); - if (!IS_INDEX(rel) || !IS_BTREE(rel)) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), @@ -232,6 +234,38 @@ bt_page_stats_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version) errmsg("block 0 is a meta page"))); CHECK_RELATION_BLOCK_RANGE(rel, blkno); +} + +/* ----------------------------------------------- + * bt_page_stats() + * + * Usage: SELECT * FROM bt_page_stats('t1_pkey', 1); + * ----------------------------------------------- + */ +static Datum +bt_page_stats_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version) +{ + text *relname = PG_GETARG_TEXT_PP(0); + int64 blkno = (ext_version == PAGEINSPECT_V1_8 ? PG_GETARG_UINT32(1) : PG_GETARG_INT64(1)); + Buffer buffer; + Relation rel; + RangeVar *relrv; + Datum result; + HeapTuple tuple; + TupleDesc tupleDesc; + int j; + char *values[11]; + BTPageStat stat; + + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to use pageinspect functions"))); + + relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname)); + rel = relation_openrv(relrv, AccessShareLock); + + bt_index_block_validate(rel, blkno); buffer = ReadBuffer(rel, blkno); LockBuffer(buffer, BUFFER_LOCK_SHARE); @@ -270,6 +304,134 @@ bt_page_stats_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version) PG_RETURN_DATUM(result); } +/* ----------------------------------------------- + * bt_multi_page_stats() + * + * Usage: SELECT * FROM bt_page_stats('t1_pkey', 1, 1); + * Where the first argument is the name followed + * by block number and number of blocks to be + * returned. + * ----------------------------------------------- + */ +Datum +bt_multi_page_stats(PG_FUNCTION_ARGS) +{ + text *relname = PG_GETARG_TEXT_PP(0); + int64 blkno = PG_GETARG_INT64(1); + int64 blk_count = PG_GETARG_INT64(2); + RangeVar *relrv; + Relation rel; + ua_page_stats *uargs; + FuncCallContext *fctx; + MemoryContext mctx; + + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to use pageinspect functions"))); + + if (SRF_IS_FIRSTCALL()) + { + fctx = SRF_FIRSTCALL_INIT(); + + relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname)); + rel = relation_openrv(relrv, AccessShareLock); + + /* + * Check if relaion is a valid btree index and block number is within + * the valid range. + */ + bt_index_block_validate(rel, blkno); + + relation_close(rel, AccessShareLock); + + mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx); + + uargs = palloc(sizeof(struct ua_page_stats)); + + /* + * Set the start block number and the total number of blocks to ne + * retrieved. + */ + uargs->blkno = blkno; + uargs->blk_count = blk_count; + + fctx->user_fctx = uargs; + + MemoryContextSwitchTo(mctx); + } + + fctx = SRF_PERCALL_SETUP(); + uargs = fctx->user_fctx; + + /* We need to fetch next block statistics */ + if (uargs->blk_count > 0) + { + Buffer buffer; + Datum result; + HeapTuple tuple; + int j; + char *values[11]; + BTPageStat stat; + TupleDesc tupleDesc; + + relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname)); + rel = relation_openrv(relrv, AccessShareLock); + + /* + * Since index may have changed between function calls, double check + * that we are still within the vaild range. + */ + CHECK_RELATION_BLOCK_RANGE(rel, uargs->blkno); + + buffer = ReadBuffer(rel, uargs->blkno); + LockBuffer(buffer, BUFFER_LOCK_SHARE); + + /* keep compiler quiet */ + stat.btpo_prev = stat.btpo_next = InvalidBlockNumber; + stat.btpo_flags = stat.free_size = stat.avg_item_size = 0; + + GetBTPageStatistics(uargs->blkno, buffer, &stat); + + UnlockReleaseBuffer(buffer); + relation_close(rel, AccessShareLock); + + /* Build a tuple descriptor for our result type */ + if (get_call_result_type(fcinfo, NULL, &tupleDesc) != TYPEFUNC_COMPOSITE) + elog(ERROR, "return type must be a row type"); + + j = 0; + values[j++] = psprintf("%u", stat.blkno); + values[j++] = psprintf("%c", stat.type); + 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.btpo_level); + values[j++] = psprintf("%d", stat.btpo_flags); + + /* Constructu tuple to be returned */ + tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc), + values); + + result = HeapTupleGetDatum(tuple); + + /* + * Move to the next block number and decrement the number of blocks + * still to be fetched + */ + uargs->blkno++; + uargs->blk_count--; + + SRF_RETURN_NEXT(fctx, result); + } + + SRF_RETURN_DONE(fctx); +} + Datum bt_page_stats_1_9(PG_FUNCTION_ARGS) { @@ -283,19 +445,6 @@ bt_page_stats(PG_FUNCTION_ARGS) return bt_page_stats_internal(fcinfo, PAGEINSPECT_V1_8); } - -/* - * cross-call data structure for SRF - */ -struct user_args -{ - Page page; - OffsetNumber offset; - bool leafpage; - bool rightmost; - TupleDesc tupd; -}; - /*------------------------------------------------------- * bt_page_print_tuples() * @@ -303,7 +452,7 @@ struct user_args * ------------------------------------------------------ */ static Datum -bt_page_print_tuples(struct user_args *uargs) +bt_page_print_tuples(struct ua_page_items *uargs) { Page page = uargs->page; OffsetNumber offset = uargs->offset; @@ -457,7 +606,7 @@ bt_page_items_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version) Datum result; FuncCallContext *fctx; MemoryContext mctx; - struct user_args *uargs; + struct ua_page_items *uargs; if (!superuser()) ereport(ERROR, @@ -477,33 +626,7 @@ bt_page_items_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version) relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname)); rel = relation_openrv(relrv, AccessShareLock); - if (!IS_INDEX(rel) || !IS_BTREE(rel)) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a %s index", - RelationGetRelationName(rel), "btree"))); - - /* - * Reject attempts to read non-local temporary relations; we would be - * likely to get wrong data since we have no visibility into the - * owning session's local buffers. - */ - if (RELATION_IS_OTHER_TEMP(rel)) - ereport(ERROR, - (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) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("block 0 is a meta page"))); - - CHECK_RELATION_BLOCK_RANGE(rel, blkno); + bt_index_block_validate(rel, blkno); buffer = ReadBuffer(rel, blkno); LockBuffer(buffer, BUFFER_LOCK_SHARE); @@ -515,7 +638,7 @@ bt_page_items_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version) */ mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx); - uargs = palloc(sizeof(struct user_args)); + uargs = palloc(sizeof(struct ua_page_items)); uargs->page = palloc(BLCKSZ); memcpy(uargs->page, BufferGetPage(buffer), BLCKSZ); @@ -591,7 +714,7 @@ bt_page_items_bytea(PG_FUNCTION_ARGS) bytea *raw_page = PG_GETARG_BYTEA_P(0); Datum result; FuncCallContext *fctx; - struct user_args *uargs; + struct ua_page_items *uargs; if (!superuser()) ereport(ERROR, @@ -607,7 +730,7 @@ bt_page_items_bytea(PG_FUNCTION_ARGS) fctx = SRF_FIRSTCALL_INIT(); mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx); - uargs = palloc(sizeof(struct user_args)); + uargs = palloc(sizeof(struct ua_page_items)); uargs->page = get_page_from_raw(raw_page); diff --git a/contrib/pageinspect/expected/btree.out b/contrib/pageinspect/expected/btree.out index 035a81a..383b9be 100644 --- a/contrib/pageinspect/expected/btree.out +++ b/contrib/pageinspect/expected/btree.out @@ -34,6 +34,117 @@ btpo_flags | 3 SELECT * FROM bt_page_stats('test1_a_idx', 2); ERROR: block number out of range +-- bt_multi_page_stats() function returns a set of records of page statistics. +CREATE TABLE test2 AS (SELECT generate_series(1, 5000) AS col1); +CREATE INDEX test2_col1_idx ON test2(col1); +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 0, 1); +ERROR: block 0 is a meta page +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 1, -1); +ERROR: block number out of range +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 1, 0); +(0 rows) + +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 1, 2); +-[ RECORD 1 ]-+----- +blkno | 1 +type | l +live_items | 367 +dead_items | 0 +avg_item_size | 16 +page_size | 8192 +free_size | 808 +btpo_prev | 0 +btpo_next | 2 +btpo_level | 0 +btpo_flags | 1 +-[ RECORD 2 ]-+----- +blkno | 2 +type | l +live_items | 367 +dead_items | 0 +avg_item_size | 16 +page_size | 8192 +free_size | 808 +btpo_prev | 1 +btpo_next | 4 +btpo_level | 0 +btpo_flags | 1 + +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 2, 6); +-[ RECORD 1 ]-+----- +blkno | 2 +type | l +live_items | 367 +dead_items | 0 +avg_item_size | 16 +page_size | 8192 +free_size | 808 +btpo_prev | 1 +btpo_next | 4 +btpo_level | 0 +btpo_flags | 1 +-[ RECORD 2 ]-+----- +blkno | 3 +type | r +live_items | 14 +dead_items | 0 +avg_item_size | 15 +page_size | 8192 +free_size | 7876 +btpo_prev | 0 +btpo_next | 0 +btpo_level | 1 +btpo_flags | 2 +-[ RECORD 3 ]-+----- +blkno | 4 +type | l +live_items | 367 +dead_items | 0 +avg_item_size | 16 +page_size | 8192 +free_size | 808 +btpo_prev | 2 +btpo_next | 5 +btpo_level | 0 +btpo_flags | 1 +-[ RECORD 4 ]-+----- +blkno | 5 +type | l +live_items | 367 +dead_items | 0 +avg_item_size | 16 +page_size | 8192 +free_size | 808 +btpo_prev | 4 +btpo_next | 6 +btpo_level | 0 +btpo_flags | 1 +-[ RECORD 5 ]-+----- +blkno | 6 +type | l +live_items | 367 +dead_items | 0 +avg_item_size | 16 +page_size | 8192 +free_size | 808 +btpo_prev | 5 +btpo_next | 7 +btpo_level | 0 +btpo_flags | 1 +-[ RECORD 6 ]-+----- +blkno | 7 +type | l +live_items | 367 +dead_items | 0 +avg_item_size | 16 +page_size | 8192 +free_size | 808 +btpo_prev | 6 +btpo_next | 8 +btpo_level | 0 +btpo_flags | 1 + +DROP TABLE test2; SELECT * FROM bt_page_items('test1_a_idx', -1); ERROR: invalid block number SELECT * FROM bt_page_items('test1_a_idx', 0); diff --git a/contrib/pageinspect/pageinspect--1.10--1.11.sql b/contrib/pageinspect/pageinspect--1.10--1.11.sql index e69de29..976d029 100644 --- a/contrib/pageinspect/pageinspect--1.10--1.11.sql +++ b/contrib/pageinspect/pageinspect--1.10--1.11.sql @@ -0,0 +1,23 @@ +/* contrib/pageinspect/pageinspect--1.10--1.11.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION pageinspect UPDATE TO '1.11'" to load this file. \quit + +-- +-- bt_multi_page_stats() +-- +CREATE FUNCTION bt_multi_page_stats(IN relname text, IN blkno int8, IN blk_count 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_level int8, + OUT btpo_flags int4) +RETURNS SETOF record +AS 'MODULE_PATHNAME', 'bt_multi_page_stats' +LANGUAGE C STRICT PARALLEL SAFE; diff --git a/contrib/pageinspect/pageinspect.control b/contrib/pageinspect/pageinspect.control index 7cdf379..f277413 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.10' +default_version = '1.11' module_pathname = '$libdir/pageinspect' relocatable = true diff --git a/contrib/pageinspect/sql/btree.sql b/contrib/pageinspect/sql/btree.sql index 1f554f0..9a95914 100644 --- a/contrib/pageinspect/sql/btree.sql +++ b/contrib/pageinspect/sql/btree.sql @@ -11,6 +11,16 @@ SELECT * FROM bt_page_stats('test1_a_idx', 0); SELECT * FROM bt_page_stats('test1_a_idx', 1); SELECT * FROM bt_page_stats('test1_a_idx', 2); +-- bt_multi_page_stats() function returns a set of records of page statistics. +CREATE TABLE test2 AS (SELECT generate_series(1, 5000) AS col1); +CREATE INDEX test2_col1_idx ON test2(col1); +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 0, 1); +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 1, -1); +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 1, 0); +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 1, 2); +SELECT * FROM bt_multi_page_stats('test2_col1_idx', 2, 6); +DROP TABLE test2; + SELECT * FROM bt_page_items('test1_a_idx', -1); SELECT * FROM bt_page_items('test1_a_idx', 0); SELECT * FROM bt_page_items('test1_a_idx', 1);