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
signature.asc
Description: PGP signature