On Sun, 31 Jul 2022 at 18:00, Hamid Akhtar <hamid.akh...@gmail.com> wrote:
> > > 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" > Attached is the rebased version of the patch (pageinspect_btree_multipagestats_03.patch) for the master branch. > >
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 9375d55..f7ec95c 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; @@ -453,7 +602,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, @@ -473,33 +622,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); @@ -511,7 +634,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); @@ -587,7 +710,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, @@ -603,7 +726,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);