Re: Reject access to unlogged tables in contrib modules

On 02/11/2026 08:38, Michael Paquier wrote:
> I have a question here. Is there a reason why you do not check after
> pg_visibility_map() and pg_visibility_map_summary()? pgstatapprox.c
> also does lookups based on block numbers and you are proposing to
> block temp and unlogged relations there. For the visibility maps, I
> doubt that it makes sense to have them return incorrect results during
> recovery. Here this means vm_readbuf() returning an invalid buffer.
> That feels confusing to me vs the choice of an error.

The only reason is that they didn't fail, so I didn't look any further..

postgres=# select * from pg_visibility_map_summary('pg_temp_1.tmp');
all_visible | all_frozen
-------------+------------
0 | 0
(1 row)

postgres=# SELECT * FROM pg_visibility_map_summary('ut');
all_visible | all_frozen
-------------+------------
0 | 0
(1 row)

...but I agree that an error message is better than incorrect results.
> Another point may be in introducing RELATION_IS_OTHER_TEMP() in
> pg_visibility. It also seems strange to not fail in this case,
> doesn't it? We would just grab incorrect data if trying to fetch VM
> information from a temp relation owned by a different session.

Agreed. I added this check in 0005.

> Rather than copy-pasting the same check N times, you could just group
> them in a shared routine and reduce the duplication. For
> pg_visibility, this could be embedded in check_relation_relkind(),
> perhaps..

Right. I just repeated an existing pattern, but I agree that it can be
done in a single place. It is indeed much better than repeating the
check multiple times. 0004 now adds it in check_relation_relkind().

Thanks!

Best, Jim