Re: Reject access to unlogged tables in contrib modules

On Mon, Oct 26, 2026 at 08:39:00AM +0100, Jim Jones wrote:
> On 26/10/2026 05:44, Michael Paquier wrote:
> > Okay, I'm sold over this set of suggestions on HEAD to improve the
> > user experience. I doubt that this is worth worrying about on stable
> > branches due to the lack of reports..
>
> +1
> I also don't think it is worth the trouble. Besides the fact that it's
> mostly a cosmetic improvement, it's not like anybody has been
> complaining too loud about it in the past years :)

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.

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.

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..
--
Michael