Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-16 Thread Nathan Bossart
RRORs in case someone invokes this function directly. * * Also, for the benefit of the pg_sequences view, we return NULL for - * unlogged sequences on standbys instead of throwing an error. + * unlogged sequences on standbys and for sequences for which we lack + * USAGE or SELECT privi

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-13 Thread Nathan Bossart
Committed. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-10 Thread Nathan Bossart
121e4751aba85a45639 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Fri, 10 May 2024 15:55:24 -0500 Subject: [PATCH v4 1/1] Fix pg_sequence_last_value() for unlogged sequences on standbys. Presently, when this function is called for an unlogged sequence on a standby server, it will

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-07 Thread Michael Paquier
On Tue, May 07, 2024 at 02:39:42PM -0500, Nathan Bossart wrote: > Okay, phew. We can still do something like v3-0002 for v18. I'll give > Michael a chance to comment on 0001 before committing/back-patching that > one. What you are doing in 0001, and 0002 for v18 sounds fine to me. -- Michael

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-07 Thread Nathan Bossart
sult = 0; /* open and lock sequence */ init_sequence(relid, , ); @@ -1792,12 +1792,22 @@ pg_sequence_last_value(PG_FUNCTION_ARGS) errmsg("permission denied for sequence %s", RelationGetRelationName(seqrel; - seq = read_seq_tuple(seqrel, , ); + /* + * For the benef

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-07 Thread Tom Lane
Nathan Bossart writes: > On Tue, May 07, 2024 at 01:44:16PM -0400, Tom Lane wrote: >> +1 to include that, as it offers a defense if someone invokes this >> function directly. In HEAD we could then rip out the test in the >> view. > I apologize for belaboring this point, but I don't see how we

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-07 Thread Nathan Bossart
ock sequence */ init_sequence(relid, , ); @@ -1792,12 +1792,22 @@ pg_sequence_last_value(PG_FUNCTION_ARGS) errmsg("permission denied for sequence %s", RelationGetRelationName(seqrel; - seq = read_seq_tuple(seqrel, , ); + /* + * For the benefit of the pg_sequences s

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-07 Thread Tom Lane
Nathan Bossart writes: > Okay, so are we okay to back-patch something like v1? Or should we also > return NULL for other sessions' temporary schemas on primaries? That would > change the condition to something like > char relpersist = seqrel->rd_rel->relpersistence; > if

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-07 Thread Nathan Bossart
On Sat, May 04, 2024 at 06:45:32PM +0900, Michael Paquier wrote: > On Fri, May 03, 2024 at 05:22:06PM -0400, Tom Lane wrote: >> Nathan Bossart writes: >>> IIUC this would cause other sessions' temporary sequences to appear in the >>> view. Is that desirable? >> >> I assume Michael meant to move

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-04 Thread Michael Paquier
On Fri, May 03, 2024 at 03:49:08PM -0500, Nathan Bossart wrote: > On Wed, May 01, 2024 at 12:39:53PM +0900, Michael Paquier wrote: >> 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()

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-04 Thread Michael Paquier
On Fri, May 03, 2024 at 05:22:06PM -0400, Tom Lane wrote: > Nathan Bossart writes: >> IIUC this would cause other sessions' temporary sequences to appear in the >> view. Is that desirable? > > I assume Michael meant to move the test into the C code, not drop > it entirely --- I agree we don't

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-03 Thread Tom Lane
Nathan Bossart writes: > On Wed, May 01, 2024 at 12:39:53PM +0900, Michael Paquier wrote: >> 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 >>

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-05-03 Thread Nathan Bossart
On Wed, May 01, 2024 at 12:39:53PM +0900, Michael Paquier wrote: > 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

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-04-30 Thread Michael Paquier
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

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-04-30 Thread Nathan Bossart
On Tue, Apr 30, 2024 at 08:13:17PM -0500, Nathan Bossart wrote: > Good point. I'll work on a patch along these lines, then. 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,

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-04-30 Thread Nathan Bossart
On Tue, Apr 30, 2024 at 09:06:04PM -0400, Tom Lane wrote: > Nathan Bossart writes: >> If you create an unlogged sequence on a primary, pg_sequence_last_value() >> for that sequence on a standby will error like so: >> postgres=# select pg_sequence_last_value('test'::regclass); >> ERROR:

Re: pg_sequence_last_value() for unlogged sequences on standbys

2024-04-30 Thread Tom Lane
Nathan Bossart writes: > If you create an unlogged sequence on a primary, pg_sequence_last_value() > for that sequence on a standby will error like so: > postgres=# select pg_sequence_last_value('test'::regclass); > ERROR: could not open file "base/5/16388": No such file or directory

pg_sequence_last_value() for unlogged sequences on standbys

2024-04-30 Thread Nathan Bossart
on is used by the pg_sequences system view, which fails with the same error on standbys. The two options I see are: * Return a better ERROR and adjust pg_sequences to avoid calling this function for unlogged sequences on standbys. * Return NULL from pg_sequence_last_value() if called for a

Re: unlogged sequences

2022-04-07 Thread Peter Eisentraut
On 06.04.22 11:12, Peter Eisentraut wrote: We could also move forward with this patch independently of the other one.  If we end up reverting the other one, then this one won't be very useful but it won't really hurt anything and it would presumably become useful eventually.  What we

Re: unlogged sequences

2022-04-06 Thread Peter Eisentraut
have some qualms. This patch is now in limbo because it appears that the logical replication of sequences feature might end up being reverted for PG15. This unlogged sequences feature is really a component of that overall feature. If we think that logical replication of sequences might stay in

Re: unlogged sequences

2022-04-04 Thread Peter Eisentraut
On 04.04.22 01:58, David G. Johnston wrote: "Because pg_dump is used to transfer data to newer versions of PostgreSQL, the output of pg_dump can be expected to load into PostgreSQL server versions newer than pg_dump's version." [1] That is what I'm getting on about when talking about

Re: unlogged sequences

2022-04-03 Thread David G. Johnston
On Sun, Apr 3, 2022 at 12:36 PM Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote: > On 03.04.22 20:50, David G. Johnston wrote: > > However, tables having an identity sequence seem to be unaddressed in > > this patch. The existing (and unchanged) pg_dump.c code results in: > > It is

Re: unlogged sequences

2022-04-03 Thread David G. Johnston
On Sun, Apr 3, 2022 at 12:36 PM Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote: > On 03.04.22 20:50, David G. Johnston wrote: > > However, tables having an identity sequence seem to be unaddressed in > > this patch. The existing (and unchanged) pg_dump.c code results in: > > It is

Re: unlogged sequences

2022-04-03 Thread Peter Eisentraut
On 03.04.22 20:50, David G. Johnston wrote: However, tables having an identity sequence seem to be unaddressed in this patch.  The existing (and unchanged) pg_dump.c code results in: It is addressed. For example, run this in PG14: create unlogged table t1 (a int generated always as identity,

Re: unlogged sequences

2022-04-03 Thread David G. Johnston
On Sun, Apr 3, 2022 at 10:19 AM Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote: > Here is an updated patch that fixes this pg_dump/pg_upgrade issue and > also adds a few more comments and documentation sentences about what > happens and what is allowed. I didn't change any

Re: unlogged sequences

2022-04-03 Thread Peter Eisentraut
1d25c999f5ab214d Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sun, 3 Apr 2022 19:14:03 +0200 Subject: [PATCH v6] Unlogged sequences Add support for unlogged sequences. Unlike for unlogged tables, this is not a performance feature. It allows sequences associated with unlogged tables t

Re: unlogged sequences

2022-04-01 Thread Robert Haas
On Fri, Apr 1, 2022 at 12:31 PM Peter Eisentraut wrote: > > An "internal" dependency means that the internal object shouldn't really > > be operated on directly. (In some cases it's allowed for convenience.) > > So I think in that case the sequence must follow the table's persistence > > in all

Re: unlogged sequences

2022-04-01 Thread David G. Johnston
On Fri, Apr 1, 2022 at 9:31 AM Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote: > On 01.04.22 18:22, Peter Eisentraut wrote: > > > > On 01.04.22 00:43, Tomas Vondra wrote: > >> Hmm, so what about doing a little bit different thing: > >> > >> 1) owned sequences inherit persistence of

Re: unlogged sequences

2022-04-01 Thread David G. Johnston
On Fri, Apr 1, 2022 at 9:22 AM Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote: > > On 01.04.22 00:43, Tomas Vondra wrote: > > Hmm, so what about doing a little bit different thing: > > > > 1) owned sequences inherit persistence of the table by default > > > > 2) allow ALTER SEQUENCE

Re: unlogged sequences

2022-04-01 Thread Peter Eisentraut
On 01.04.22 18:22, Peter Eisentraut wrote: On 01.04.22 00:43, Tomas Vondra wrote: Hmm, so what about doing a little bit different thing: 1) owned sequences inherit persistence of the table by default 2) allow ALTER SEQUENCE to change persistence for all sequences (no restriction for owned

Re: unlogged sequences

2022-04-01 Thread Peter Eisentraut
On 01.04.22 00:43, Tomas Vondra wrote: Hmm, so what about doing a little bit different thing: 1) owned sequences inherit persistence of the table by default 2) allow ALTER SEQUENCE to change persistence for all sequences (no restriction for owned sequences) 3) ALTER TABLE ... SET [UN]LOGGED

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
On Thu, Mar 31, 2022 at 6:03 PM Robert Haas wrote: > On Thu, Mar 31, 2022 at 8:44 PM David G. Johnston > wrote: > > > The "give the user power" argument is also valid. But since they > already have power through unowned sequences, having the owned sequences > more narrowly defined doesn't

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
On Thu, Mar 31, 2022 at 6:03 PM Robert Haas wrote: > > In this new system, does the user still get a logged sequence? If they > get an unlogged sequence, how does dump-and-restore work? What if they > want to still have a logged sequence? But for sequences that are > simply owned, there is no

Re: unlogged sequences

2022-03-31 Thread Robert Haas
On Thu, Mar 31, 2022 at 8:44 PM David G. Johnston wrote: > It seems reasonable to extend the definition of "ownership of a sequence" in > this way. We always let you create unowned sequences with whatever > persistence you like if you need flexibility. I'd say it doesn't seem to have any

Re: unlogged sequences

2022-03-31 Thread Robert Haas
On Thu, Mar 31, 2022 at 8:42 PM Tomas Vondra wrote: > Well, yeah. I did this because the patch was somewhat inconsistent when > handling owned sequences - it updated persistence for owned sequences > when persistence for the table changed, expecting to keep them in sync, > but then it also

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
On Thu, Mar 31, 2022 at 5:25 PM Robert Haas wrote: > On Thu, Mar 31, 2022 at 10:14 AM Tomas Vondra > wrote: > > * When linking a sequence to a table (ALTER SEQUENCE ... OWNED BY), > > there's an ereport(ERROR) if the relpersistence values do not match. > > > > * Disallow changing persistence

Re: unlogged sequences

2022-03-31 Thread Tomas Vondra
On 4/1/22 02:25, Robert Haas wrote: > On Thu, Mar 31, 2022 at 10:14 AM Tomas Vondra > wrote: >> * When linking a sequence to a table (ALTER SEQUENCE ... OWNED BY), >> there's an ereport(ERROR) if the relpersistence values do not match. >> >> * Disallow changing persistence for owned sequences

Re: unlogged sequences

2022-03-31 Thread Robert Haas
On Thu, Mar 31, 2022 at 10:14 AM Tomas Vondra wrote: > * When linking a sequence to a table (ALTER SEQUENCE ... OWNED BY), > there's an ereport(ERROR) if the relpersistence values do not match. > > * Disallow changing persistence for owned sequences directly. Wait, what? I don't understand why

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
remise of not changing any existing behavior, is: > > > > DB State: Allow a sequence to be unlogged. > > Command: ALTER SEQUENCE SET UNLOGGED > > Limitation: The above command fails if the sequence is unowned, or it is > > owned and the table owning it is not UNLOGGED > &

Re: unlogged sequences

2022-03-31 Thread Tomas Vondra
QUENCE SET UNLOGGED > Limitation: The above command fails if the sequence is unowned, or it is > owned and the table owning it is not UNLOGGED > > (optional safety) Limitation: Changing a table from unlogged to logged > while owning unlogged sequences would be prohibited > (option

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
On Thu, Mar 31, 2022 at 1:40 PM David G. Johnston < david.g.johns...@gmail.com> wrote: > The DBA just has to execute the ALTER SEQUENCE command on all relevant > sequences. > Additional, if we do not implement the forced matching of persistence mode, we should consider adding an "ALTER TABLE SET

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
(optional safety) Limitation: Changing a table from unlogged to logged while owning unlogged sequences would be prohibited (optional safety) Compensatory Behavior: Add the ALTER SEQUENCE SET LOGGED command for owned sequences to get them logged again in preparation for changing the table to being logged.

Re: unlogged sequences

2022-03-31 Thread Tomas Vondra
hange, exactly? To create the sequences as UNLOGGED, but > we'd not update the persistence after that? > > > Today, a newly created unlogged table with an automatically owned > sequence (serial, generated identity) has a logged sequence.  This patch > changes that so the new auto

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
To create the sequences as UNLOGGED, but > we'd not update the persistence after that? > > Today, a newly created unlogged table with an automatically owned sequence (serial, generated identity) has a logged sequence. This patch changes that so the new automatically owned sequence is unlogged.

Re: unlogged sequences

2022-03-31 Thread Tomas Vondra
On 3/31/22 19:35, David G. Johnston wrote: > On Thu, Mar 31, 2022 at 9:28 AM Andres Freund > wrote: > > I agree it makes sense to have logged sequences with unlogged tables. We > should call out the behavioural change somewhere prominent in the > release >

Re: unlogged sequences

2022-03-31 Thread David G. Johnston
On Thu, Mar 31, 2022 at 9:28 AM Andres Freund wrote: > I agree it makes sense to have logged sequences with unlogged tables. We > should call out the behavioural change somewhere prominent in the release > notes. > > We can/do already support that unlikely use case by allowing one to remove the

Re: unlogged sequences

2022-03-31 Thread Andres Freund
Hi, On 2022-03-31 16:14:25 +0200, Tomas Vondra wrote: > 1) Do we need to do something about pg_upgrade? I mean, we did not have > unlogged sequences until now, so existing databases may have unlogged > tables with logged sequences. If people run pg_upgrade, what should be > the end re

Re: unlogged sequences

2022-03-31 Thread Tomas Vondra
. * Disallow changing persistence for owned sequences directly. But I wonder about two things: 1) Do we need to do something about pg_upgrade? I mean, we did not have unlogged sequences until now, so existing databases may have unlogged tables with logged sequences. If people run pg_upgrade, what

Re: unlogged sequences

2022-03-29 Thread Peter Eisentraut
red it would be suitable to dust off this old discussion on unlogged sequences, mainly so that sequences attached to unlogged tables can be excluded from replication. Attached is a new patch that incorporates the above suggestions, with some slight refactoring.  The only thing I didn't/couldn't

Re: unlogged sequences

2022-03-24 Thread Peter Eisentraut
end of fill_seq_with_data() if writing INIT_FORKNUM. The if (RelationNeedsWAL(rel)) would need an || forkNum == INIT_FORKNUM. Now that logical replication of sequences is nearing completion, I figured it would be suitable to dust off this old discussion on unlogged sequences, mainly so that sequences attac

Re: unlogged sequences

2022-02-28 Thread Peter Eisentraut
red it would be suitable to dust off this old discussion on unlogged sequences, mainly so that sequences attached to unlogged tables can be excluded from replication. Attached is a new patch that incorporates the above suggestions, with some slight refactoring.  The only thing I didn't/couldn't

Re: unlogged sequences

2022-02-11 Thread Peter Eisentraut
add a FlushBuffer() to the end of fill_seq_with_data() if writing INIT_FORKNUM. The if (RelationNeedsWAL(rel)) would need an || forkNum == INIT_FORKNUM. Now that logical replication of sequences is nearing completion, I figured it would be suitable to dust off this old discussion on unlogged sequen

Re: unlogged sequences

2019-09-10 Thread Alvaro Herrera from 2ndQuadrant
On 2019-Aug-01, Thomas Munro wrote: > On Wed, Jun 26, 2019 at 6:38 AM Andres Freund wrote: > > On 2019-06-20 09:30:34 +0200, Peter Eisentraut wrote: > > > I'm looking for feedback from those who have worked on tableam and > > > storage manager to see what the right interfaces are or whether some

Re: unlogged sequences

2019-08-01 Thread Thomas Munro
On Wed, Jun 26, 2019 at 6:38 AM Andres Freund wrote: > On 2019-06-20 09:30:34 +0200, Peter Eisentraut wrote: > > I'm looking for feedback from those who have worked on tableam and > > storage manager to see what the right interfaces are or whether some new > > interfaces might perhaps be

Re: unlogged sequences

2019-06-25 Thread Andres Freund
am design matters much around sequences? To me it's a historical accident that sequences kinda look like tables, not more. > + /* > + * create init fork for unlogged sequences > + * > + * The logic follows that of RelationCreateStorage() and > +

Re: unlogged sequences

2019-06-23 Thread Peter Eisentraut
On 2019-06-21 07:31, Michael Paquier wrote: > 1) Some SQL queries: > create unlogged sequence popo; > alter sequence popo increment 2; The problem is that the above command does a relation rewrite but the code doesn't know to copy the init fork of the sequence. That will need to be addressed. >

Re: unlogged sequences

2019-06-20 Thread Michael Paquier
unlogged. > But there is no support for unlogged sequences, so I looked into that. Thanks for doing so. > If you copy the initial sequence relation file to the init fork, then > this all seems to work out just fine. Attached is a patch. The > low-level copying seems to be handled quite inconsistently

unlogged sequences

2019-06-20 Thread Peter Eisentraut
The discussion in bug #15631 revealed that serial/identity sequences of temporary tables should really also be temporary (easy), and that serial/identity sequences of unlogged tables should also be unlogged. But there is no support for unlogged sequences, so I looked into that. If you copy