Code review for btreefuncs additions: restrict to superusers to avoid

exposing user data to others, and clean up usage of deprecated APIs.

contrib/pageinspect/README.pageinspect

@@ -1,5 +1,6 @@
 The functions in this module allow you to inspect the contents of data pages
-at a low level, for debugging purposes.
+at a low level, for debugging purposes.  All of these functions may be used
+only by superusers.
 
 1. Installation
 
@@ -13,12 +14,12 @@ at a low level, for debugging purposes.
     ------------
     get_raw_page reads one block of the named table and returns a copy as a
     bytea field. This allows a single time-consistent copy of the block to be
-    made. Use of this functions is restricted to superusers.
+    made.
 
     page_header
     -----------
     page_header shows fields which are common to all PostgreSQL heap and index
-    pages. Use of this function is restricted to superusers.
+    pages.
 
     A page image obtained with get_raw_page should be passed as argument:
 
@@ -36,8 +37,7 @@ at a low level, for debugging purposes.
     heap_page_items shows all line pointers on a heap page.  For those line
     pointers that are in use, tuple headers are also shown. All tuples are
     shown, whether or not the tuples were visible to an MVCC snapshot at the
-    time the raw page was copied. Use of this function is restricted to
+    time the raw page was copied.
-    superusers.
 
     A heap page image obtained with get_raw_page should be passed as argument:
 
@@ -48,7 +48,7 @@ at a low level, for debugging purposes.
 
     bt_metap
     --------
-    bt_metap() returns information about the btree index metapage:
+    bt_metap() returns information about a btree index's metapage:
 
         test=> SELECT * FROM bt_metap('pg_cast_oid_index');
         -[ RECORD 1 ]-----

contrib/pageinspect/btreefuncs.c

@@ -24,36 +24,24 @@
 
 #include "postgres.h"
 
-#include "fmgr.h"
-#include "funcapi.h"
 #include "access/heapam.h"
-#include "access/itup.h"
 #include "access/nbtree.h"
-#include "access/transam.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_type.h"
+#include "funcapi.h"
+#include "miscadmin.h"
 #include "utils/builtins.h"
-#include "utils/inval.h"
 
-PG_FUNCTION_INFO_V1(bt_metap);
-PG_FUNCTION_INFO_V1(bt_page_items);
-PG_FUNCTION_INFO_V1(bt_page_stats);
 
 extern Datum bt_metap(PG_FUNCTION_ARGS);
 extern Datum bt_page_items(PG_FUNCTION_ARGS);
 extern Datum bt_page_stats(PG_FUNCTION_ARGS);
 
-#define BTMETAP_TYPE "public.bt_metap_type"
+PG_FUNCTION_INFO_V1(bt_metap);
-#define BTMETAP_NCOLUMNS 6
+PG_FUNCTION_INFO_V1(bt_page_items);
+PG_FUNCTION_INFO_V1(bt_page_stats);
 
-#define BTPAGEITEMS_TYPE "public.bt_page_items_type"
+#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
-#define BTPAGEITEMS_NCOLUMNS 6
-
-#define BTPAGESTATS_TYPE "public.bt_page_stats_type"
-#define BTPAGESTATS_NCOLUMNS 11
-
-
-#define IS_INDEX(r) ((r)->rd_rel->relkind == 'i')
 #define IS_BTREE(r) ((r)->rd_rel->relam == BTREE_AM_OID)
 
 #define CHECK_PAGE_OFFSET_RANGE(pg, offnum) { \
@@ -93,42 +81,11 @@ typedef struct BTPageStat
 	BTCycleId	btpo_cycleid;
 }	BTPageStat;
 
-/* ------------------------------------------------
- * A structure for a whole btree index statistics
- * used by pgstatindex().
- * ------------------------------------------------
- */
-typedef struct BTIndexStat
-{
-	uint32		magic;
-	uint32		version;
-	BlockNumber root_blkno;
-	uint32		level;
-
-	BlockNumber fastroot;
-	uint32		fastlevel;
-
-	uint32		live_items;
-	uint32		dead_items;
-
-	uint32		root_pages;
-	uint32		internal_pages;
-	uint32		leaf_pages;
-	uint32		empty_pages;
-	uint32		deleted_pages;
-
-	uint32		page_size;
-	uint32		avg_item_size;
-
-	uint32		max_avail;
-	uint32		free_space;
-}	BTIndexStat;
-
 
 /* -------------------------------------------------
  * GetBTPageStatistics()
  *
- * Collect statistics of single b-tree leaf page
+ * Collect statistics of single b-tree page
  * -------------------------------------------------
  */
 static void
@@ -199,7 +156,7 @@ GetBTPageStatistics(BlockNumber blkno, Buffer buffer, BTPageStat * stat)
 /* -----------------------------------------------
  * bt_page()
  *
- * Usage: SELECT * FROM bt_page('t1_pkey', 0);
+ * Usage: SELECT * FROM bt_page('t1_pkey', 1);
  * -----------------------------------------------
  */
 Datum
@@ -208,31 +165,33 @@ bt_page_stats(PG_FUNCTION_ARGS)
 	text	   *relname = PG_GETARG_TEXT_P(0);
 	uint32		blkno = PG_GETARG_UINT32(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);
 
-	CHECK_RELATION_BLOCK_RANGE(rel, blkno);
-
-	buffer = ReadBuffer(rel, blkno);
-
 	if (!IS_INDEX(rel) || !IS_BTREE(rel))
-		elog(ERROR, "bt_page_stats() can only be used on b-tree index");
+		elog(ERROR, "relation \"%s\" is not a btree index",
+			 RelationGetRelationName(rel));
 
 	if (blkno == 0)
 		elog(ERROR, "block 0 is a meta page");
 
-	{
+	CHECK_RELATION_BLOCK_RANGE(rel, blkno);
-		HeapTuple	tuple;
-		TupleDesc	tupleDesc;
-		int			j;
-		char	   *values[BTPAGESTATS_NCOLUMNS];
 
-		BTPageStat	stat;
+	buffer = ReadBuffer(rel, blkno);
 
 	/* keep compiler quiet */
 	stat.btpo_prev = stat.btpo_next = InvalidBlockNumber;
@@ -240,12 +199,13 @@ bt_page_stats(PG_FUNCTION_ARGS)
 
 	GetBTPageStatistics(blkno, buffer, &stat);
 
-		tupleDesc = RelationNameGetTupleDesc(BTPAGESTATS_TYPE);
+	/* 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] = palloc(32);
 	snprintf(values[j++], 32, "%d", stat.blkno);
-
 	values[j] = palloc(32);
 	snprintf(values[j++], 32, "%c", stat.type);
 	values[j] = palloc(32);
@@ -262,21 +222,18 @@ bt_page_stats(PG_FUNCTION_ARGS)
 	snprintf(values[j++], 32, "%d", stat.btpo_prev);
 	values[j] = palloc(32);
 	snprintf(values[j++], 32, "%d", stat.btpo_next);
-
 	values[j] = palloc(32);
 	if (stat.type == 'd')
 		snprintf(values[j++], 32, "%d", stat.btpo.xact);
 	else
 		snprintf(values[j++], 32, "%d", stat.btpo.level);
-
 	values[j] = palloc(32);
 	snprintf(values[j++], 32, "%d", stat.btpo_flags);
 
 	tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
 								   values);
 
-		result = TupleGetDatum(TupleDescGetSlot(tupleDesc), tuple);
+	result = HeapTupleGetDatum(tuple);
-	}
 
 	ReleaseBuffer(buffer);
 
@@ -288,22 +245,19 @@ bt_page_stats(PG_FUNCTION_ARGS)
 /*-------------------------------------------------------
  * bt_page_items()
  *
- * Get IndexTupleData set in a leaf page
+ * Get IndexTupleData set in a btree page
  *
- * Usage: SELECT * FROM bt_page_items('t1_pkey', 0);
+ * Usage: SELECT * FROM bt_page_items('t1_pkey', 1);
  *-------------------------------------------------------
  */
-/* ---------------------------------------------------
+
- * data structure for SRF to hold a scan information
+/*
- * ---------------------------------------------------
+ * cross-call data structure for SRF
  */
 struct user_args
 {
-	TupleDesc	tupd;
-	Relation	rel;
-	Buffer		buffer;
 	Page		page;
-	uint16		offset;
+	OffsetNumber offset;
 };
 
 Datum
@@ -311,49 +265,72 @@ bt_page_items(PG_FUNCTION_ARGS)
 {
 	text	   *relname = PG_GETARG_TEXT_P(0);
 	uint32		blkno = PG_GETARG_UINT32(1);
-
-	RangeVar   *relrv;
 	Datum		result;
-	char	   *values[BTPAGEITEMS_NCOLUMNS];
+	char	   *values[6];
-	BTPageOpaque opaque;
 	HeapTuple	tuple;
-	ItemId		id;
-
 	FuncCallContext *fctx;
 	MemoryContext mctx;
-	struct user_args *uargs = NULL;
+	struct user_args *uargs;
+
+	if (!superuser())
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 (errmsg("must be superuser to use pageinspect functions"))));
+
+	if (SRF_IS_FIRSTCALL())
+	{
+		RangeVar   *relrv;
+		Relation	rel;
+		Buffer		buffer;
+		BTPageOpaque opaque;
+		TupleDesc	tupleDesc;
+
+		fctx = SRF_FIRSTCALL_INIT();
+
+		relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
+		rel = relation_openrv(relrv, AccessShareLock);
+
+		if (!IS_INDEX(rel) || !IS_BTREE(rel))
+			elog(ERROR, "relation \"%s\" is not a btree index",
+				 RelationGetRelationName(rel));
 
 		if (blkno == 0)
 			elog(ERROR, "block 0 is a meta page");
 
-	if (SRF_IS_FIRSTCALL())
+		CHECK_RELATION_BLOCK_RANGE(rel, blkno);
-	{
+
-		fctx = SRF_FIRSTCALL_INIT();
+		buffer = ReadBuffer(rel, blkno);
+
+		/*
+		 * We copy the page into local storage to avoid holding pin on
+		 * the buffer longer than we must, and possibly failing to
+		 * release it at all if the calling query doesn't fetch all rows.
+		 */
 		mctx = MemoryContextSwitchTo(fctx->multi_call_memory_ctx);
 
 		uargs = palloc(sizeof(struct user_args));
 
-		uargs->tupd = RelationNameGetTupleDesc(BTPAGEITEMS_TYPE);
+		uargs->page = palloc(BLCKSZ);
+		memcpy(uargs->page, BufferGetPage(buffer), BLCKSZ);
+
+		ReleaseBuffer(buffer);
+		relation_close(rel, AccessShareLock);
+
 		uargs->offset = FirstOffsetNumber;
 
-		relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
-		uargs->rel = relation_openrv(relrv, AccessShareLock);
-
-		CHECK_RELATION_BLOCK_RANGE(uargs->rel, blkno);
-
-		uargs->buffer = ReadBuffer(uargs->rel, blkno);
-
-		if (!IS_INDEX(uargs->rel) || !IS_BTREE(uargs->rel))
-			elog(ERROR, "bt_page_items() can only be used on b-tree index");
-
-		uargs->page = BufferGetPage(uargs->buffer);
-
 		opaque = (BTPageOpaque) PageGetSpecialPointer(uargs->page);
 
 		if (P_ISDELETED(opaque))
 			elog(NOTICE, "page is deleted");
 
 		fctx->max_calls = PageGetMaxOffsetNumber(uargs->page);
+
+		/* 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");
+
+		fctx->attinmeta = TupleDescGetAttInMetadata(tupleDesc);
+
 		fctx->user_fctx = uargs;
 
 		MemoryContextSwitchTo(mctx);
@@ -364,7 +341,13 @@ bt_page_items(PG_FUNCTION_ARGS)
 
 	if (fctx->call_cntr < fctx->max_calls)
 	{
+		ItemId		id;
 		IndexTuple	itup;
+		int			j;
+		int			off;
+		int			dlen;
+		char	   *dump;
+		char	   *ptr;
 
 		id = PageGetItemId(uargs->page, uargs->offset);
 
@@ -373,15 +356,13 @@ bt_page_items(PG_FUNCTION_ARGS)
 
 		itup = (IndexTuple) PageGetItem(uargs->page, id);
 
-		{
+		j = 0;
-			int			j = 0;
-
-			BlockNumber blkno = BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid));
-
 		values[j] = palloc(32);
 		snprintf(values[j++], 32, "%d", uargs->offset);
 		values[j] = palloc(32);
-			snprintf(values[j++], 32, "(%u,%u)", blkno, itup->t_tid.ip_posid);
+		snprintf(values[j++], 32, "(%u,%u)",
+				 BlockIdGetBlockNumber(&(itup->t_tid.ip_blkid)),
+				 itup->t_tid.ip_posid);
 		values[j] = palloc(32);
 		snprintf(values[j++], 32, "%d", (int) IndexTupleSize(itup));
 		values[j] = palloc(32);
@@ -389,34 +370,20 @@ bt_page_items(PG_FUNCTION_ARGS)
 		values[j] = palloc(32);
 		snprintf(values[j++], 32, "%c", IndexTupleHasVarwidths(itup) ? 't' : 'f');
 
-			{
+		ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info);
-				int			off;
+		dlen = IndexTupleSize(itup) - IndexInfoFindDataOffset(itup->t_info);
-				char	   *dump;
+		dump = palloc0(dlen * 3 + 1);
-				char	   *ptr = (char *) itup + IndexInfoFindDataOffset(itup->t_info);
-
-				dump = palloc(IndexTupleSize(itup) * 3);
-				memset(dump, 0, IndexTupleSize(itup) * 3);
-
-				for (off = 0;
-					 off < IndexTupleSize(itup) - IndexInfoFindDataOffset(itup->t_info);
-					 off++)
-				{
-					if (dump[0] == '\0')
-						sprintf(dump, "%02x", *(ptr + off) & 0xff);
-					else
-					{
-						char		buf[4];
-
-						sprintf(buf, " %02x", *(ptr + off) & 0xff);
-						strcat(dump, buf);
-					}
-				}
 		values[j] = dump;
+		for (off = 0; off < dlen; off++)
+		{
+			if (off > 0)
+				*dump++ = ' ';
+			sprintf(dump, "%02x", *(ptr + off) & 0xff);
+			dump += 2;
 		}
 
-			tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(uargs->tupd), values);
+		tuple = BuildTupleFromCStrings(fctx->attinmeta, values);
-			result = TupleGetDatum(TupleDescGetSlot(uargs->tupd), tuple);
+		result = HeapTupleGetDatum(tuple);
-		}
 
 		uargs->offset = uargs->offset + 1;
 
@@ -424,9 +391,8 @@ bt_page_items(PG_FUNCTION_ARGS)
 	}
 	else
 	{
-		ReleaseBuffer(uargs->buffer);
+		pfree(uargs->page);
-		relation_close(uargs->rel, AccessShareLock);
+		pfree(uargs);
-
 		SRF_RETURN_DONE(fctx);
 	}
 }
@@ -435,7 +401,7 @@ bt_page_items(PG_FUNCTION_ARGS)
 /* ------------------------------------------------
  * bt_metap()
  *
- * Get a btree meta-page information
+ * Get a btree's meta-page information
  *
  * Usage: SELECT * FROM bt_metap('t1_pkey')
  * ------------------------------------------------
@@ -444,33 +410,36 @@ Datum
 bt_metap(PG_FUNCTION_ARGS)
 {
 	text	   *relname = PG_GETARG_TEXT_P(0);
-	Buffer		buffer;
+	Datum		result;
-
 	Relation	rel;
 	RangeVar   *relrv;
-	Datum		result;
+	BTMetaPageData *metad;
+	TupleDesc	tupleDesc;
+	int			j;
+	char	   *values[6];
+	Buffer		buffer;
+	Page		page;
+	HeapTuple	tuple;
+
+	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))
-		elog(ERROR, "bt_metap() can only be used on b-tree index");
+		elog(ERROR, "relation \"%s\" is not a btree index",
+			 RelationGetRelationName(rel));
 
 	buffer = ReadBuffer(rel, 0);
-
+	page = BufferGetPage(buffer);
-	{
-		BTMetaPageData *metad;
-
-		TupleDesc	tupleDesc;
-		int			j;
-		char	   *values[BTMETAP_NCOLUMNS];
-		HeapTuple	tuple;
-
-		Page		page = BufferGetPage(buffer);
-
 	metad = BTPageGetMeta(page);
 
-		tupleDesc = RelationNameGetTupleDesc(BTMETAP_TYPE);
+	/* 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] = palloc(32);
@@ -489,8 +458,7 @@ bt_metap(PG_FUNCTION_ARGS)
 	tuple = BuildTupleFromCStrings(TupleDescGetAttInMetadata(tupleDesc),
 								   values);
 
-		result = TupleGetDatum(TupleDescGetSlot(tupleDesc), tuple);
+	result = HeapTupleGetDatum(tuple);
-	}
 
 	ReleaseBuffer(buffer);
 

contrib/pageinspect/pageinspect.sql.in

@@ -12,98 +12,80 @@ LANGUAGE C STRICT;
 --
 -- page_header()
 --
-CREATE TYPE page_header_type AS (
+CREATE OR REPLACE FUNCTION page_header(IN page bytea,
-	lsn text,
+    OUT lsn text,
-	tli smallint,
+    OUT tli smallint,
-	flags smallint,
+    OUT flags smallint,
-	lower smallint,
+    OUT lower smallint,
-	upper smallint,
+    OUT upper smallint,
-	special smallint,
+    OUT special smallint,
-	pagesize smallint,
+    OUT pagesize smallint,
-	version smallint
+    OUT version smallint)
-);
-
-CREATE OR REPLACE FUNCTION page_header(bytea)
-RETURNS page_header_type
 AS 'MODULE_PATHNAME', 'page_header'
 LANGUAGE C STRICT;
 
 --
 -- heap_page_items()
 --
-CREATE TYPE heap_page_items_type AS (
+CREATE OR REPLACE FUNCTION heap_page_items(IN page bytea,
-	lp smallint,
+    OUT lp smallint,
-	lp_off smallint,
+    OUT lp_off smallint,
-	lp_flags smallint,
+    OUT lp_flags smallint,
-	lp_len smallint,
+    OUT lp_len smallint,
-	t_xmin xid,
+    OUT t_xmin xid,
-	t_xmax xid,
+    OUT t_xmax xid,
-	t_field3 int4,
+    OUT t_field3 int4,
-	t_ctid tid,
+    OUT t_ctid tid,
-	t_infomask2 smallint,
+    OUT t_infomask2 smallint,
-	t_infomask smallint,
+    OUT t_infomask smallint,
-	t_hoff smallint,
+    OUT t_hoff smallint,
-	t_bits text,
+    OUT t_bits text,
-	t_oid oid
+    OUT t_oid oid)
-);
+RETURNS SETOF record
-
-CREATE OR REPLACE FUNCTION heap_page_items(bytea)
-RETURNS SETOF heap_page_items_type
 AS 'MODULE_PATHNAME', 'heap_page_items'
 LANGUAGE C STRICT;
 
 --
 -- bt_metap()
 --
-CREATE TYPE bt_metap_type AS (
+CREATE OR REPLACE FUNCTION bt_metap(IN relname text,
-  magic int4,
+    OUT magic int4,
-  version int4,
+    OUT version int4,
-  root int4,
+    OUT root int4,
-  level int4,
+    OUT level int4,
-  fastroot int4,
+    OUT fastroot int4,
-  fastlevel int4
+    OUT fastlevel int4)
-);
-
-CREATE OR REPLACE FUNCTION bt_metap(text)
-RETURNS bt_metap_type
 AS 'MODULE_PATHNAME', 'bt_metap'
-LANGUAGE 'C' STRICT;
+LANGUAGE C STRICT;
 
 --
 -- bt_page_stats()
 --
-CREATE TYPE bt_page_stats_type AS (
+CREATE OR REPLACE FUNCTION bt_page_stats(IN relname text, IN blkno int4,
-  blkno int4,
+    OUT blkno int4,
-  type char,
+    OUT type "char",
-  live_items int4,
+    OUT live_items int4,
-  dead_items int4,
+    OUT dead_items int4,
-  avg_item_size float,
+    OUT avg_item_size int4,
-  page_size int4,
+    OUT page_size int4,
-  free_size int4,
+    OUT free_size int4,
-  btpo_prev int4,
+    OUT btpo_prev int4,
-  btpo_next int4,
+    OUT btpo_next int4,
-  btpo int4,
+    OUT btpo int4,
-  btpo_flags int4
+    OUT btpo_flags int4)
-);
-
-CREATE OR REPLACE FUNCTION bt_page_stats(text, int4)
-RETURNS bt_page_stats_type
 AS 'MODULE_PATHNAME', 'bt_page_stats'
-LANGUAGE 'C' STRICT;
+LANGUAGE C STRICT;
 
 --
 -- bt_page_items()
 --
-CREATE TYPE bt_page_items_type AS (
+CREATE OR REPLACE FUNCTION bt_page_items(IN relname text, IN blkno int4,
-  itemoffset smallint,
+    OUT itemoffset smallint,
-  ctid tid,
+    OUT ctid tid,
-  itemlen smallint,
+    OUT itemlen smallint,
-  nulls bool,
+    OUT nulls bool,
-  vars bool,
+    OUT vars bool,
-  data text
+    OUT data text)
-);
+RETURNS SETOF record
-
-CREATE OR REPLACE FUNCTION bt_page_items(text, int4)
-RETURNS SETOF bt_page_items_type
 AS 'MODULE_PATHNAME', 'bt_page_items'
-LANGUAGE 'C' STRICT;
+LANGUAGE C STRICT;

contrib/pageinspect/uninstall_pageinspect.sql

@@ -2,19 +2,8 @@
 SET search_path = public;
 
 DROP FUNCTION get_raw_page(text, int4);
-
 DROP FUNCTION page_header(bytea);
-DROP TYPE page_header_type;
-
 DROP FUNCTION heap_page_items(bytea);
-DROP TYPE heap_page_items_type;
-
 DROP FUNCTION bt_metap(text);
-DROP TYPE bt_metap_type;
-
 DROP FUNCTION bt_page_stats(text, int4);
-DROP TYPE bt_page_stats_type;
-
 DROP FUNCTION bt_page_items(text, int4);
-DROP TYPE bt_page_items_type;
-