From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: pgsql: Prevent instability in contrib/pageinspect's regression test.
Date: Mon, 21 Nov 2022 14:12:49 -0500
Message-ID: <3258427.1669057969@sss.pgh.pa.us> (raw)
In-Reply-To: <3219281.1669054138@sss.pgh.pa.us>
References: <E1ox94a-000EHu-VH@gemulon.postgresql.org>
<20221121172206.h6kywxrzqcmhoqom@awork3.anarazel.de>
<3214376.1669051457@sss.pgh.pa.us>
<3215927.1669052115@sss.pgh.pa.us>
<CA+TgmoaYMUv1ab2vAh7AoSUqUYHtPx9Cmehue2=JCXO3gvsDbw@mail.gmail.com>
<3219281.1669054138@sss.pgh.pa.us>
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
Attachments:fix-pageinspect-parallel-markings.patch (text/x-diff, 3.2K) downloaddiff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefileindex 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.buildindex 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.sqlnew file mode 100644index 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.controlindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, robertmhaas@gmail.com, andres@anarazel.de
Subject: Re: pgsql: Prevent instability in contrib/pageinspect's regression test.
In-Reply-To: <3258427.1669057969@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox