On Wed, Feb 23, 2022 at 12:09:02PM +0500, Daria Lepikhova wrote: > And one more addition. In the previous version of the patch, I forgot to add > tests for the gist index, but the described problem is also relevant for it.
So, I have looked at this second part of the thread, and concluded that we should not fail for empty pages. First, we fetch pages from the buffer pool in normal mode, where empty pages are valid. There is also a second point in favor of doing so: code paths dedicated to hash indexes already do that, marking such pages as simply "unused". The proposal from Julien upthread sounds cleaner to me though in the long run, as NULL gives the user the possibility to do a full-table scan with simple clauses to filter out anything found as NULL. Painting more PageIsNew() across the place requires a bit more work than a simple ereport(ERROR) in get_page_from_raw(), of course, but the gain is the portability of the functions. (One can have a lot of fun playing with random inputs and breaking most code paths, but that's not new.) -- Michael
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index bf12901ac3..87e75f663c 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -58,6 +58,9 @@ brin_page_type(PG_FUNCTION_ARGS)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
switch (BrinPageType(page))
{
case BRIN_PAGETYPE_META:
@@ -86,6 +89,9 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
{
Page page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ return page;
+
/* verify the special space says this page is what we want */
if (BrinPageType(page) != type)
ereport(ERROR,
@@ -138,6 +144,13 @@ brin_page_items(PG_FUNCTION_ARGS)
/* minimally verify the page we got */
page = verify_brin_page(raw_page, BRIN_PAGETYPE_REGULAR, "regular");
+ if (PageIsNew(page))
+ {
+ brin_free_desc(bdesc);
+ index_close(indexRel, AccessShareLock);
+ PG_RETURN_NULL();
+ }
+
/*
* Initialize output functions for all indexed datatypes; simplifies
* calling them later.
@@ -313,6 +326,9 @@ brin_metapage_info(PG_FUNCTION_ARGS)
page = verify_brin_page(raw_page, BRIN_PAGETYPE_META, "metapage");
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
/* Build a tuple descriptor for our result type */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
@@ -364,6 +380,12 @@ brin_revmap_data(PG_FUNCTION_ARGS)
/* minimally verify the page we got */
page = verify_brin_page(raw_page, BRIN_PAGETYPE_REVMAP, "revmap");
+ if (PageIsNew(page))
+ {
+ MemoryContextSwitchTo(mctx);
+ PG_RETURN_NULL();
+ }
+
state = palloc(sizeof(*state));
state->tids = ((RevmapContents *) PageGetContents(page))->rm_tids;
state->idx = 0;
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index d9628dd664..dde640fd33 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -611,6 +611,12 @@ bt_page_items_bytea(PG_FUNCTION_ARGS)
uargs->page = get_page_from_raw(raw_page);
+ if (PageIsNew(uargs->page))
+ {
+ MemoryContextSwitchTo(mctx);
+ PG_RETURN_NULL();
+ }
+
uargs->offset = FirstOffsetNumber;
opaque = (BTPageOpaque) PageGetSpecialPointer(uargs->page);
diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out
index 10cd36c177..15786a0c0d 100644
--- a/contrib/pageinspect/expected/brin.out
+++ b/contrib/pageinspect/expected/brin.out
@@ -52,4 +52,29 @@ SELECT * FROM brin_page_items(get_raw_page('test1_a_idx', 2), 'test1_a_idx')
CREATE INDEX test1_a_btree ON test1 (a);
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree');
ERROR: "test1_a_btree" is not a BRIN index
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT brin_page_type(decode(repeat('00', :block_size), 'hex'));
+ brin_page_type
+----------------
+
+(1 row)
+
+SELECT brin_page_items(decode(repeat('00', :block_size), 'hex'), 'test1_a_idx');
+ brin_page_items
+-----------------
+(0 rows)
+
+SELECT brin_metapage_info(decode(repeat('00', :block_size), 'hex'));
+ brin_metapage_info
+--------------------
+
+(1 row)
+
+SELECT brin_revmap_data(decode(repeat('00', :block_size), 'hex'));
+ brin_revmap_data
+------------------
+
+(1 row)
+
DROP TABLE test1;
diff --git a/contrib/pageinspect/expected/btree.out b/contrib/pageinspect/expected/btree.out
index 80b3dfe861..8815b56b15 100644
--- a/contrib/pageinspect/expected/btree.out
+++ b/contrib/pageinspect/expected/btree.out
@@ -85,4 +85,10 @@ ERROR: "test1_a_hash" is not a btree index
SELECT bt_page_items('aaa'::bytea);
ERROR: invalid page size
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT bt_page_items(decode(repeat('00', :block_size), 'hex'));
+-[ RECORD 1 ]-+-
+bt_page_items |
+
DROP TABLE test1;
diff --git a/contrib/pageinspect/expected/gin.out b/contrib/pageinspect/expected/gin.out
index 802f48284b..ce36175366 100644
--- a/contrib/pageinspect/expected/gin.out
+++ b/contrib/pageinspect/expected/gin.out
@@ -47,3 +47,17 @@ ERROR: invalid page size
SELECT gin_page_opaque_info('ccc'::bytea);
ERROR: invalid page size
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT gin_leafpage_items(decode(repeat('00', :block_size), 'hex'));
+-[ RECORD 1 ]------+-
+gin_leafpage_items |
+
+SELECT gin_metapage_info(decode(repeat('00', :block_size), 'hex'));
+-[ RECORD 1 ]-----+-
+gin_metapage_info |
+
+SELECT gin_page_opaque_info(decode(repeat('00', :block_size), 'hex'));
+-[ RECORD 1 ]--------+-
+gin_page_opaque_info |
+
diff --git a/contrib/pageinspect/expected/gist.out b/contrib/pageinspect/expected/gist.out
index 3f33e04066..648f740500 100644
--- a/contrib/pageinspect/expected/gist.out
+++ b/contrib/pageinspect/expected/gist.out
@@ -79,4 +79,22 @@ ERROR: invalid page size
SELECT gist_page_opaque_info('aaa'::bytea);
ERROR: invalid page size
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT gist_page_items_bytea(decode(repeat('00', :block_size), 'hex'));
+ gist_page_items_bytea
+-----------------------
+(0 rows)
+
+SELECT gist_page_items(decode(repeat('00', :block_size), 'hex'), 'test_gist_idx'::regclass);
+ gist_page_items
+-----------------
+(0 rows)
+
+SELECT gist_page_opaque_info(decode(repeat('00', :block_size), 'hex'));
+ gist_page_opaque_info
+-----------------------
+
+(1 row)
+
DROP TABLE test_gist;
diff --git a/contrib/pageinspect/expected/hash.out b/contrib/pageinspect/expected/hash.out
index 6c606630dd..4727b2ffe9 100644
--- a/contrib/pageinspect/expected/hash.out
+++ b/contrib/pageinspect/expected/hash.out
@@ -180,4 +180,16 @@ ERROR: invalid page size
SELECT hash_page_type('ddd'::bytea);
ERROR: invalid page size
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT hash_metapage_info(decode(repeat('00', :block_size), 'hex'));
+ERROR: page is not a hash meta page
+SELECT hash_page_items(decode(repeat('00', :block_size), 'hex'));
+ERROR: page is not a hash bucket or overflow page
+SELECT hash_page_stats(decode(repeat('00', :block_size), 'hex'));
+ERROR: page is not a hash bucket or overflow page
+SELECT hash_page_type(decode(repeat('00', :block_size), 'hex'));
+-[ RECORD 1 ]--+-------
+hash_page_type | unused
+
DROP TABLE test_hash;
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 9bbdda7f18..7a21fc9ffc 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -218,3 +218,23 @@ ERROR: invalid page size
SELECT page_header('ccc'::bytea);
ERROR: invalid page size
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT fsm_page_contents(decode(repeat('00', :block_size), 'hex'));
+ fsm_page_contents
+-------------------
+
+(1 row)
+
+SELECT page_header(decode(repeat('00', :block_size), 'hex'));
+ page_header
+-----------------------
+ (0/0,0,0,0,0,0,0,0,0)
+(1 row)
+
+SELECT page_checksum(decode(repeat('00', :block_size), 'hex'), 1);
+ page_checksum
+---------------
+
+(1 row)
+
diff --git a/contrib/pageinspect/fsmfuncs.c b/contrib/pageinspect/fsmfuncs.c
index b914da1d4a..2b9679e454 100644
--- a/contrib/pageinspect/fsmfuncs.c
+++ b/contrib/pageinspect/fsmfuncs.c
@@ -46,6 +46,10 @@ fsm_page_contents(PG_FUNCTION_ARGS)
errmsg("must be superuser to use raw page functions")));
page = get_page_from_raw(raw_page);
+
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
fsmpage = (FSMPage) PageGetContents(page);
initStringInfo(&sinfo);
diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c
index f55128857e..d8825bccb2 100644
--- a/contrib/pageinspect/ginfuncs.c
+++ b/contrib/pageinspect/ginfuncs.c
@@ -49,6 +49,9 @@ gin_metapage_info(PG_FUNCTION_ARGS)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
opaq = (GinPageOpaque) PageGetSpecialPointer(page);
if (opaq->flags != GIN_META)
ereport(ERROR,
@@ -107,6 +110,9 @@ gin_page_opaque_info(PG_FUNCTION_ARGS)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
opaq = (GinPageOpaque) PageGetSpecialPointer(page);
/* Build a tuple descriptor for our result type */
@@ -184,6 +190,12 @@ gin_leafpage_items(PG_FUNCTION_ARGS)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ {
+ MemoryContextSwitchTo(mctx);
+ PG_RETURN_NULL();
+ }
+
if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GinPageOpaqueData)))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index a31cff47fe..4a93b38d7f 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -55,6 +55,9 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
opaq = (GISTPageOpaque) PageGetSpecialPointer(page);
/* Build a tuple descriptor for our result type */
@@ -113,6 +116,9 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
/* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */
if (GistPageIsDeleted(page))
elog(NOTICE, "page is deleted");
@@ -185,6 +191,12 @@ gist_page_items(PG_FUNCTION_ARGS)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ {
+ index_close(indexRel, AccessShareLock);
+ PG_RETURN_NULL();
+ }
+
/* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */
if (GistPageIsDeleted(page))
elog(NOTICE, "page is deleted");
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 92ffb2d930..730a46b1d8 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -352,6 +352,9 @@ page_checksum_internal(PG_FUNCTION_ARGS, enum pageinspect_version ext_version)
page = get_page_from_raw(raw_page);
+ if (PageIsNew(page))
+ PG_RETURN_NULL();
+
PG_RETURN_INT16(pg_checksum_page((char *) page, blkno));
}
diff --git a/contrib/pageinspect/sql/brin.sql b/contrib/pageinspect/sql/brin.sql
index 8717229c5d..4e967a1fc5 100644
--- a/contrib/pageinspect/sql/brin.sql
+++ b/contrib/pageinspect/sql/brin.sql
@@ -19,4 +19,11 @@ SELECT * FROM brin_page_items(get_raw_page('test1_a_idx', 2), 'test1_a_idx')
CREATE INDEX test1_a_btree ON test1 (a);
SELECT brin_page_items(get_raw_page('test1_a_btree', 0), 'test1_a_btree');
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT brin_page_type(decode(repeat('00', :block_size), 'hex'));
+SELECT brin_page_items(decode(repeat('00', :block_size), 'hex'), 'test1_a_idx');
+SELECT brin_metapage_info(decode(repeat('00', :block_size), 'hex'));
+SELECT brin_revmap_data(decode(repeat('00', :block_size), 'hex'));
+
DROP TABLE test1;
diff --git a/contrib/pageinspect/sql/btree.sql b/contrib/pageinspect/sql/btree.sql
index fdda777b9e..ce6e7c9982 100644
--- a/contrib/pageinspect/sql/btree.sql
+++ b/contrib/pageinspect/sql/btree.sql
@@ -34,4 +34,8 @@ SELECT bt_page_items('test1_a_hash', 0);
SELECT bt_page_items('aaa'::bytea);
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT bt_page_items(decode(repeat('00', :block_size), 'hex'));
+
DROP TABLE test1;
diff --git a/contrib/pageinspect/sql/gin.sql b/contrib/pageinspect/sql/gin.sql
index aadb07856d..6b128f3848 100644
--- a/contrib/pageinspect/sql/gin.sql
+++ b/contrib/pageinspect/sql/gin.sql
@@ -28,3 +28,9 @@ SELECT gin_leafpage_items('aaa'::bytea);
SELECT gin_metapage_info('bbb'::bytea);
SELECT gin_page_opaque_info('ccc'::bytea);
\set VERBOSITY default
+
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT gin_leafpage_items(decode(repeat('00', :block_size), 'hex'));
+SELECT gin_metapage_info(decode(repeat('00', :block_size), 'hex'));
+SELECT gin_page_opaque_info(decode(repeat('00', :block_size), 'hex'));
diff --git a/contrib/pageinspect/sql/gist.sql b/contrib/pageinspect/sql/gist.sql
index 8abeb19140..6a4274b4e3 100644
--- a/contrib/pageinspect/sql/gist.sql
+++ b/contrib/pageinspect/sql/gist.sql
@@ -39,4 +39,10 @@ SELECT gist_page_items('aaa'::bytea, 'test_gist_idx'::regclass);
SELECT gist_page_opaque_info('aaa'::bytea);
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT gist_page_items_bytea(decode(repeat('00', :block_size), 'hex'));
+SELECT gist_page_items(decode(repeat('00', :block_size), 'hex'), 'test_gist_idx'::regclass);
+SELECT gist_page_opaque_info(decode(repeat('00', :block_size), 'hex'));
+
DROP TABLE test_gist;
diff --git a/contrib/pageinspect/sql/hash.sql b/contrib/pageinspect/sql/hash.sql
index fcddd706ae..3cb7940513 100644
--- a/contrib/pageinspect/sql/hash.sql
+++ b/contrib/pageinspect/sql/hash.sql
@@ -92,4 +92,11 @@ SELECT hash_page_stats('ccc'::bytea);
SELECT hash_page_type('ddd'::bytea);
\set VERBOSITY default
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT hash_metapage_info(decode(repeat('00', :block_size), 'hex'));
+SELECT hash_page_items(decode(repeat('00', :block_size), 'hex'));
+SELECT hash_page_stats(decode(repeat('00', :block_size), 'hex'));
+SELECT hash_page_type(decode(repeat('00', :block_size), 'hex'));
+
DROP TABLE test_hash;
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index 38b1681541..2dda6895bf 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -91,3 +91,9 @@ SELECT fsm_page_contents('aaa'::bytea);
SELECT page_checksum('bbb'::bytea, 0);
SELECT page_header('ccc'::bytea);
\set VERBOSITY default
+
+-- Test for empty pages with raw input.
+SHOW block_size \gset
+SELECT fsm_page_contents(decode(repeat('00', :block_size), 'hex'));
+SELECT page_header(decode(repeat('00', :block_size), 'hex'));
+SELECT page_checksum(decode(repeat('00', :block_size), 'hex'), 1);
signature.asc
Description: PGP signature
