On Tue, Apr 30, 2024 at 09:05:31PM -0500, Nathan Bossart wrote:
> This ended up being easier than I expected. While unlogged sequences are
> only supported on v15 and above, temporary sequences have been around since
> v7.2, so this will probably need to be back-patched to all supported
> versions.
Unlogged and temporary relations cannot be accessed during recovery,
so I'm OK with your change to force a NULL for both relpersistences.
However, it seems to me that you should also drop the
pg_is_other_temp_schema() in system_views.sql for the definition of
pg_sequences. Doing that on HEAD now would be OK, but there's nothing
urgent to it so it may be better done once v18 opens up. Note that
pg_is_other_temp_schema() is only used for this sequence view, which
is a nice cleanup.
By the way, shouldn't we also change the function to return NULL for a
failed permission check? It would be possible to remove the
has_sequence_privilege() as well, thanks to that, and a duplication
between the code and the function view. I've been looking around a
bit, noticing one use of this function in check_pgactivity (nagios
agent), and its query also has a has_sequence_privilege() so returning
NULL would simplify its definition in the long-run. I'd suspect other
monitoring queries to do something similar to bypass permission
errors.
> The added test case won't work for v12-v14 since it uses an
> unlogged sequence.
That would require a BackgroundPsql to maintain the connection to the
primary, so not having a test is OK by me.
--
Michael