Re: pgsql: Prevent instability in contrib/pageinspect's regression test. - Mailing list pgsql-hackers

From Tom Lane
Subject Re: pgsql: Prevent instability in contrib/pageinspect's regression test.
Date
Msg-id 3258427.1669057969@sss.pgh.pa.us
Whole thread Raw
In response to Re: pgsql: Prevent instability in contrib/pageinspect's regression test.  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> I'll check to see if any sibling functions have the same issue,
> and push a patch to adjust them.
> Presumably the parallel labeling has to be fixed as far back as
> it's marked that way (didn't look).  Maybe we should push the
> test change further back too, just to exercise this.

Hmm, so this is easy enough to fix in HEAD and v15, as attached.
However, there's a problem in older branches: their newest
versions of pageinspect are older than 1.10, so this doesn't
work as-is.

We could imagine inventing versions like 1.9.1, and providing
a script pageinspect--1.9--1.9.1.sql to do what's done here
as well as (in later branches) pageinspect--1.9.1--1.10.sql that
duplicates pageinspect--1.9--1.10.sql, and then the same again for
1.8 and 1.7 for the older in-support branches.  That seems like
an awful lot of trouble for something that there have been no
field complaints about.

I'm inclined to apply this in HEAD and v15, and revert the
test change in v14, and call it good.

            regards, tom lane

diff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index 5c0736564a..ad5a3ac511 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@ -13,7 +13,8 @@ OBJS = \
     rawpage.o

 EXTENSION = pageinspect
-DATA =  pageinspect--1.9--1.10.sql pageinspect--1.8--1.9.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 \
diff --git a/contrib/pageinspect/meson.build b/contrib/pageinspect/meson.build
index 3ec50b9445..25fa7dc20c 100644
--- a/contrib/pageinspect/meson.build
+++ b/contrib/pageinspect/meson.build
@@ -33,6 +33,7 @@ install_data(
   'pageinspect--1.7--1.8.sql',
   'pageinspect--1.8--1.9.sql',
   'pageinspect--1.9--1.10.sql',
+  'pageinspect--1.10--1.11.sql',
   'pageinspect.control',
   kwargs: contrib_data_args,
 )
diff --git a/contrib/pageinspect/pageinspect--1.10--1.11.sql b/contrib/pageinspect/pageinspect--1.10--1.11.sql
new file mode 100644
index 0000000000..8fa5e105bc
--- /dev/null
+++ b/contrib/pageinspect/pageinspect--1.10--1.11.sql
@@ -0,0 +1,28 @@
+/* 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
+
+--
+-- Functions that fetch relation pages must be PARALLEL RESTRICTED,
+-- not PARALLEL SAFE, otherwise they will fail when run on a
+-- temporary table in a parallel worker process.
+--
+
+ALTER FUNCTION get_raw_page(text, int8) PARALLEL RESTRICTED;
+ALTER FUNCTION get_raw_page(text, text, int8) PARALLEL RESTRICTED;
+-- tuple_data_split must be restricted because it may fetch TOAST data.
+ALTER FUNCTION tuple_data_split(oid, bytea, integer, integer, text) PARALLEL RESTRICTED;
+ALTER FUNCTION tuple_data_split(oid, bytea, integer, integer, text, bool) PARALLEL RESTRICTED;
+-- heap_page_item_attrs must be restricted because it calls tuple_data_split.
+ALTER FUNCTION heap_page_item_attrs(bytea, regclass, bool) PARALLEL RESTRICTED;
+ALTER FUNCTION heap_page_item_attrs(bytea, regclass) PARALLEL RESTRICTED;
+ALTER FUNCTION bt_metap(text) PARALLEL RESTRICTED;
+ALTER FUNCTION bt_page_stats(text, int8) PARALLEL RESTRICTED;
+ALTER FUNCTION bt_page_items(text, int8) PARALLEL RESTRICTED;
+ALTER FUNCTION hash_bitmap_info(regclass, int8) PARALLEL RESTRICTED;
+-- brin_page_items might be parallel safe, because it seems to touch
+-- only index metadata, but I don't think there's a point in risking it.
+-- Likewise for gist_page_items.
+ALTER FUNCTION brin_page_items(bytea, regclass) PARALLEL RESTRICTED;
+ALTER FUNCTION gist_page_items(bytea, regclass) PARALLEL RESTRICTED;
diff --git a/contrib/pageinspect/pageinspect.control b/contrib/pageinspect/pageinspect.control
index 7cdf37913d..f277413dd8 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

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Damage control for planner's get_actual_variable_endpoint() runaway
Next
From: Tom Lane
Date:
Subject: Re: Re: Damage control for planner's get_actual_variable_endpoint() runaway