Reject COPY TO of unlogged relations during recovery

Nice catch!

Claude pointed out that a few contrib modules are also affected (pgstattuple, pg_visibility, pageinspect and pgrowlocks)

I think the location is right for the COPY TO case, but maybe it would be the best to handle all locations in the same patch(set)?

+		foreach_oid(child, children)
+		{
+			if (get_rel_persistence(child) != RELPERSISTENCE_PERMANENT)

Can we put this check in the existing loop above? And maybe the error detail could also name the specific relation?

Hi Zsolt

On 15/10/2026 23:38, Zsolt Parragi wrote:

Claude pointed out that a few contrib modules are also affected
(pgstattuple, pg_visibility, pageinspect and pgrowlocks)

I think the location is right for the COPY TO case, but maybe it would
be the best to handle all locations in the same patch(set)?

The fix itself here shouldn't be too difficult, but I anticipate some
questions on "how" unlogged tables should be treated, for instance
amcheck seems to simply skips it:

postgres=# set client_min_messages to debug1;
SET
postgres=# select * from verify_heapam('ut');
DEBUG: cannot verify unlogged relation "ut" during recovery, skipping
blkno | offnum | attnum | msg
-------+--------+--------+-----
(0 rows)

So I am not sure how the mentioned contrib modules should behave. But I
agree that anything is better than the current error message:

postgres=# select * from pgrowlocks('ut');
ERROR: could not open file "base/5/16410": No such file or directory

That said, I would be in favour of creating a separate patch for the
contrib modules. WDYT?

+		foreach_oid(child, children)
+		{
+			if (get_rel_persistence(child) != RELPERSISTENCE_PERMANENT)

Can we put this check in the existing loop above? And maybe the error
detail could also name the specific relation?

I moved the check to the existing loop and added the relation name to
errdetail on partition tables:

== primary ==

psql (20devel)
Type "help" for help.

postgres=# create unlogged table ut as select 1 ;
SELECT 1
postgres=# create table pt (a int) partition by range (a);
CREATE TABLE
postgres=# create unlogged table pt_p1 partition of pt for values from
(0) to (10);
CREATE TABLE

== standby ==

psql (20devel)
Type "help" for help.

postgres=# copy ut to stdout;
ERROR: cannot access temporary or unlogged relations during recovery
postgres=# copy pt to stdout;
ERROR: cannot access temporary or unlogged relations during recovery
DETAIL: Partition "pt_p1" is an unlogged table in partitioned table "pt".

I didn't add the relation name to non-partitioned unlogged tables to
make it consistent with the errmsg of a select:

postgres=# select * from ut;
ERROR: cannot access temporary or unlogged relations during recovery

PFA v2.

Thanks for the review!

Best, Jim

On 16/10/2026 09:34, Jim Jones wrote:

So I am not sure how the mentioned contrib modules should behave. But I
agree that anything is better than the current error message:

postgres=# select * from pgrowlocks('ut');
ERROR: could not open file "base/5/16410": No such file or directory

That said, I would be in favour of creating a separate patch for the
contrib modules. WDYT?

I took a look at the contrib modules and the fix is pretty much
identical everywhere. Regarding how to approach it, it I guess where a
function fails today with "could not open file", it's fine to report a
proper error message instead -- this rather unhelpful error message is
sitting there for a long time and didn't seem to bother anybody.

Thoughts?

Best, Jim

On Fri, Oct 16, 2026 at 4:06 AM Jim Jones <jim.jones@uni-muenster.de> wrote:

On 16/10/2026 09:34, Jim Jones wrote:

So I am not sure how the mentioned contrib modules should behave. But I
agree that anything is better than the current error message:

postgres=# select * from pgrowlocks('ut');
ERROR: could not open file "base/5/16410": No such file or directory

That said, I would be in favour of creating a separate patch for the
contrib modules. WDYT?

I took a look at the contrib modules and the fix is pretty much
identical everywhere. Regarding how to approach it, it I guess where a
function fails today with "could not open file", it's fine to report a
proper error message instead -- this rather unhelpful error message is
sitting there for a long time and didn't seem to bother anybody.

Thoughts?

While I agree that we can report a proper error message instead at
these places, I think we need to note that pageinspect's check for
init fork should work even on the standby.

Also, currtid2() seems to have the same issue.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On 16/10/2026 20:11, Masahiko Sawada wrote:

While I agree that we can report a proper error message instead at
these places, I think we need to note that pageinspect's check for
init fork should work even on the standby.

Ok, I will create a followup thread only with the contrib fixes and take
this into account for pageinspect. Thanks.

Also, currtid2() seems to have the same issue.

Yeah, I can reproduce the same issue in currtid2().

psql (20devel)
Type "help" for help.

postgres=# SELECT currtid2('t', '(0,1)'::tid);
ERROR: could not open file "base/5/16384": No such file or directory

0002 attached adds a check to report an error message for it.

Thanks!

Best, Jim

On 17/10/2026 13:01, Jim Jones wrote:

Ok, I will create a followup thread only with the contrib fixes and take
this into account for pageinspect. Thanks.

I created another CF entry dedicated to the contrib modules:

https://commitfest.postgresql.org/patch/7478/

I also added a small test into 001_stream_rep.pl to check the currti2 fix.

Thanks!

Best, Jim

On Sun, Oct 18, 2026 at 7:14 AM Jim Jones <jim.jones@uni-muenster.de> wrote:

On 17/10/2026 13:01, Jim Jones wrote:

Ok, I will create a followup thread only with the contrib fixes and take
this into account for pageinspect. Thanks.

These patches look good to me. Both patches should be backpatched to
v14. I'll push them, barring objections.

I created another CF entry dedicated to the contrib modules:

https://commitfest.postgresql.org/patch/7478/

Thanks. I'll look at it.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On 10/20/26 10:52 PM, Masahiko Sawada wrote:

These patches look good to me. Both patches should be backpatched to
v14. I'll push them, barring objections.

Thanks!

Best, Jim

On Tue, Oct 20, 2026 at 3:12 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

On 10/20/26 10:52 PM, Masahiko Sawada wrote:

These patches look good to me. Both patches should be backpatched to
v14. I'll push them, barring objections.

Thanks!

I realized that currtid2() is an internal undocumented function so we
don't necessarily need to have the regression tests. So I've pushed
the patches after removing the regression test.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com