unlogged sequences

On Thu, Jun 20, 2019 at 09:30:34AM +0200, Peter Eisentraut wrote:

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.

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 across the
code, so I'm not sure what the most appropriate way to do this would be.
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 appropriate.

But the actual deal is that the sequence meta-data is now in
pg_sequences and not the init forks, no? I have just done a small
test:
1) Some SQL queries:
create unlogged sequence popo;
alter sequence popo increment 2;
select nextval('popo');
select nextval('popo');
2) Then a hard crash:
pg_ctl stop -m immediate
pg_ctl start
3) Again, with a crash:
select nextval('popo');
#2 0x000055ce60f3208d in ExceptionalCondition
(conditionName=0x55ce610f0570 "!(((PageHeader) (page))->pd_special >=
(__builtin_offsetof (PageHeaderData, pd_linp)))",
errorType=0x55ce610f0507 "FailedAssertion",
fileName=0x55ce610f04e0 "../../../src/include/storage/bufpage.h",
lineNumber=317) at assert.c:54
#3 0x000055ce60b43200 in PageValidateSpecialPointer
(page=0x7ff7692b3d80 "") at
../../../src/include/storage/bufpage.h:317
#4 0x000055ce60b459d4 in read_seq_tuple (rel=0x7ff768ad27e0,
buf=0x7ffc5707f0bc, seqdatatuple=0x7ffc5707f0a0) at
sequence.c:1213
#5 0x000055ce60b447ee in nextval_internal (relid=16385,
check_permissions=true) at sequence.c:678
#6 0x000055ce60b44533 in nextval_oid (fcinfo=0x55ce62537570) at sequence.c:607
--
Michael

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.

select nextval('popo');
select nextval('popo');
2) Then a hard crash:
pg_ctl stop -m immediate
pg_ctl start
3) Again, with a crash:
select nextval('popo');
#2 0x000055ce60f3208d in ExceptionalCondition

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi,

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

Hm, it's not clear to me that tableam 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
+	 * RelationCopyStorage().
+	 */
+	if (seq->sequence->relpersistence == RELPERSISTENCE_UNLOGGED)
+	{
+		SMgrRelation srel;
+		PGAlignedBlock buf;
+		Page		page = (Page) buf.data;
+
+		FlushRelationBuffers(rel);

That's pretty darn expensive, especially when we just need to flush out
a *single* page, as it needs to scan all of shared buffers. Seems better
to just to initialize the page from scratch? Any reason not to do that?

+		srel = smgropen(rel->rd_node, InvalidBackendId);
+		smgrcreate(srel, INIT_FORKNUM, false);
+		log_smgrcreate(&rel->rd_node, INIT_FORKNUM);
+
+		Assert(smgrnblocks(srel, MAIN_FORKNUM) == 1);
+
+		smgrread(srel, MAIN_FORKNUM, 0, buf.data);
+
+		if (!PageIsVerified(page, 0))
+			ereport(ERROR,
+					(errcode(ERRCODE_DATA_CORRUPTED),
+					 errmsg("invalid page in block %u of relation %s",
+							0,
+							relpathbackend(srel->smgr_rnode.node,
+										   srel->smgr_rnode.backend,
+										   MAIN_FORKNUM))));
+
+		log_newpage(&srel->smgr_rnode.node, INIT_FORKNUM, 0, page, false);
+		PageSetChecksumInplace(page, 0);
+		smgrextend(srel, INIT_FORKNUM, 0, buf.data, false);
+		smgrclose(srel);
+	}

I.e. I think it'd be better if we just added a fork argument to
fill_seq_with_data(), and then do something like

smgrcreate(srel, INIT_FORKNUM, false);
log_smgrcreate(&rel->rd_node, INIT_FORKNUM);
fill_seq_with_data(rel, tuple, INIT_FORKNUM);

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

Alternatively you could just copy the contents from the buffer currently
filled in fill_seq_with_data() to the main fork, and do a memcpy. But
that seems unnecessarily complicated, because you'd again need to do WAL
logging etc.

Greetings,

Andres Freund

On Wed, Jun 26, 2019 at 6:38 AM Andres Freund <andres@anarazel.de> 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 appropriate.

[lots of feedback that requires making decisions]

Seems to be actively under development but no new patch yet. Moved to next CF.

--
Thomas Munro
https://enterprisedb.com

On 2019-Aug-01, Thomas Munro wrote:

On Wed, Jun 26, 2019 at 6:38 AM Andres Freund <andres@anarazel.de> 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 appropriate.

[lots of feedback that requires making decisions]

Seems to be actively under development but no new patch yet. Moved to next CF.

Marked Waiting on Author.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 25.06.19 20:37, Andres Freund wrote:

I.e. I think it'd be better if we just added a fork argument to
fill_seq_with_data(), and then do something like

smgrcreate(srel, INIT_FORKNUM, false);
log_smgrcreate(&rel->rd_node, INIT_FORKNUM);
fill_seq_with_data(rel, tuple, INIT_FORKNUM);

and 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
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 do was to
call FlushBuffers(), since that is not an exported function. So this
still calls FlushRelationBuffers(), which was previously not liked.
Ideas welcome.

I have also re-tested the crash reported by Michael Paquier in the old
discussion and added test cases that catch them.

The rest of the patch is just documentation, DDL support, client
support, etc.

What is not done yet is support for ALTER SEQUENCE ... SET
LOGGED/UNLOGGED. This is a bit of a problem because:

1. The new behavior is that a serial/identity sequence of a new unlogged
table is now also unlogged.
2. There is also a new restriction that changing a table to logged is
not allowed if it is linked to an unlogged sequence. (This is IMO
similar to the existing restriction on linking mixed logged/unlogged
tables via foreign keys.)
3. Thus, currently, you can't create an unlogged table with a
serial/identity column and then change it to logged. This is reflected
in some of the test changes I had to make in alter_table.sql to work
around this. These should eventually go away.

Interestingly, there is grammar support for ALTER SEQUENCE ... SET
LOGGED/UNLOGGED because there is this:

| ALTER SEQUENCE qualified_name alter_table_cmds
{
AlterTableStmt *n = makeNode(AlterTableStmt);
n->relation = $3;
n->cmds = $4;
n->objtype = OBJECT_SEQUENCE;
n->missing_ok = false;
$$ = (Node *)n;
}

But it is rejected later in tablecmds.c. In fact, it appears that this
piece of grammar is currently useless because there are no
alter_table_cmds that actually work for sequences. (This used to be
different because things like OWNER TO also went through here.)

I tried to make tablecmds.c handle sequences as well, but that became
messy. So I'm thinking about making ALTER SEQUENCE ... SET
LOGGED/UNLOGGED an entirely separate code path and rip out the above
grammar, but that needs some further pondering.

But all that is a bit of a separate effort, so in the meantime some
review of the changes in and around fill_seq_with_data() would be useful.

rebased patch, no functional changes

Here is an updated patch that now also includes SET LOGGED/UNLOGGED
support. So this version addresses all known issues and open problems.

Patch rebased over some conflicts, and some tests simplified.

Hi,

Here's a slightly improved patch, adding a couple checks and tests for
owned sequences to ensure both objects have the same persistence. In
particular:

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

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 should be
the end result? Should it convert the sequences to unlogged ones, should
it fail and force the user to fix this manually, or what?

2) Does it actually make sense to force owned sequences to have the same
relpersistence as the table? I can imagine use cases where it's OK to
discard and recalculate the data, but I'd still want to ensure unique
IDs. Like some data loads, for example.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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 result? Should it convert the sequences to unlogged ones, should
it fail and force the user to fix this manually, or what?

2) Does it actually make sense to force owned sequences to have the same
relpersistence as the table? I can imagine use cases where it's OK to
discard and recalculate the data, but I'd still want to ensure unique
IDs. Like some data loads, for example.

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.

I don't think we should make pg_upgrade change the loggedness of sequences.

Greetings,

Andres Freund

On Thu, Mar 31, 2022 at 9:28 AM Andres Freund <andres@anarazel.de> 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 OWNERSHIP dependency between the table and the sequence.

I'm fine with owned sequences tracking the persistence attribute of the
owning table.

I don't think we should make pg_upgrade change the loggedness of sequences.

We are willing to change the default behavior here so it is going to affect
dump/restore anyway, might as well fully commit and do the same for
pg_upgrade. The vast majority of users will benefit from the new default
behavior.

I don't actually get, though, how that would play with pg_dump since it
always emits an unowned, and thus restored as logged, sequence first and
then alters the sequence to be owned by the table. Thus restoring an old
SQL dump into the v15 is going to fail if we prohibit
unlogged-table/logged-sequence; unless we actively change the logged-ness
of the sequence when subordinating it to a table.

Thus, the choices seem to be:

1) implement forced persistence agreement for owned sequences, changing the
sequence to match the table when the alter table happens, and during
pg_upgrade.
2) do not force persistence agreement for owned sequences

If choosing option 2, are you on board with changing the behavior of CREATE
UNLOGGED TABLE with respect to any auto-generated sequences?

David J.

On 3/31/22 19:35, David G. Johnston wrote:

On Thu, Mar 31, 2022 at 9:28 AM Andres Freund <andres@anarazel.de
<mailto:andres@anarazel.de>> 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.

I'm not sure I follow. If we allow logged sequences with unlogged
tables, there's be no behavioral change, no?

We can/do already support that unlikely use case by allowing one to
remove the OWNERSHIP dependency between the table and the sequence.

I'm fine with owned sequences tracking the persistence attribute of the
owning table.

So essentially an independent sequence, used in a default value.

I don't think we should make pg_upgrade change the loggedness of
sequences.

We are willing to change the default behavior here so it is going to
affect dump/restore anyway, might as well fully commit and do the same
for pg_upgrade.  The vast majority of users will benefit from the new
default behavior.

Whatever we do, I think we should keep the pg_dump and pg_upgrade
behavior as consistent as possible.

I don't actually get, though, how that would play with pg_dump since it
always emits an unowned, and thus restored as logged, sequence first and
then alters the sequence to be owned by the table.  Thus restoring an
old SQL dump into the v15 is going to fail if we prohibit
unlogged-table/logged-sequence; unless we actively change the
logged-ness of the sequence when subordinating it to a table.

Yeah. I guess we'd need to either automatically switch the sequence to
the right persistence when linking it to the table, or maybe we could
modify pg_dump to emit UNLOGGED when the table is unlogged (but that
would work only when using the new pg_dump).

Thus, the choices seem to be:

1) implement forced persistence agreement for owned sequences, changing
the sequence to match the table when the alter table happens, and during
pg_upgrade.
2) do not force persistence agreement for owned sequences

If choosing option 2, are you on board with changing the behavior of
CREATE UNLOGGED TABLE with respect to any auto-generated sequences?

What behavior change, exactly? To create the sequences as UNLOGGED, but
we'd not update the persistence after that?

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Mar 31, 2022 at 12:36 PM Tomas Vondra <tomas.vondra@enterprisedb.com>
wrote:

On 3/31/22 19:35, David G. Johnston wrote:

On Thu, Mar 31, 2022 at 9:28 AM Andres Freund <andres@anarazel.de
<mailto:andres@anarazel.de>> 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.

I'm not sure I follow. If we allow logged sequences with unlogged
tables, there's be no behavioral change, no?

As noted below, the behavior change is in how CREATE TABLE behaves. Not
whether or not mixed persistence is allowed.

or maybe we could
modify pg_dump to emit UNLOGGED when the table is unlogged (but that
would work only when using the new pg_dump).

Yes, the horse has already left the barn. I don't really have an opinion
on whether to leave the barn door open or closed.

If choosing option 2, are you on board with changing the behavior of
CREATE UNLOGGED TABLE with respect to any auto-generated sequences?

What behavior change, 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 automatically owned sequence is unlogged. This seems to be
generally agreed upon as being desirable - but given the fact that unlogged
tables will not have unlogged sequences it seems worth confirming that this
minor inconsistency is acceptable.

The first newly added behavior is just allowing sequences to be unlogged.
That is the only mandatory feature introduced by this patch and doesn't
seem contentious.

The second newly added behavior being proposed is to have the persistence
of the sequence be forcibly matched to the table. Whether this is
desirable is the main point under discussion.

David J.

On 3/31/22 21:55, David G. Johnston wrote:

On Thu, Mar 31, 2022 at 12:36 PM Tomas Vondra
<tomas.vondra@enterprisedb.com <mailto:tomas.vondra@enterprisedb.com>>
wrote:

On 3/31/22 19:35, David G. Johnston wrote:

On Thu, Mar 31, 2022 at 9:28 AM Andres Freund <andres@anarazel.de

<mailto:andres@anarazel.de>

<mailto:andres@anarazel.de <mailto:andres@anarazel.de>>> 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.

I'm not sure I follow. If we allow logged sequences with unlogged
tables, there's be no behavioral change, no?

As noted below, the behavior change is in how CREATE TABLE behaves.  Not
whether or not mixed persistence is allowed.
 

or maybe we could
modify pg_dump to emit UNLOGGED when the table is unlogged (but that
would work only when using the new pg_dump).

Yes, the horse has already left the barn.  I don't really have an
opinion on whether to leave the barn door open or closed.
 

If choosing option 2, are you on board with changing the behavior of
CREATE UNLOGGED TABLE with respect to any auto-generated sequences?

What behavior change, 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 automatically owned sequence is unlogged.  This
seems to be generally agreed upon as being desirable - but given the
fact that unlogged tables will not have unlogged sequences it seems
worth confirming that this minor inconsistency is acceptable.

The first newly added behavior is just allowing sequences to be
unlogged.  That is the only mandatory feature introduced by this patch
and doesn't seem contentious.

The second newly added behavior being proposed is to have the
persistence of the sequence be forcibly matched to the table.  Whether
this is desirable is the main point under discussion.

Right. The latest version of the patch also prohibits changing
persistence of owned sequences directly. But that's probably considered
to be part of the second behavior.

I agree the first part is not contentious, so shall we extract this part
of the patch and get that committed for PG15? Or is that too late to
make such changes to the patch?

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Mar 31, 2022 at 1:05 PM Tomas Vondra <tomas.vondra@enterprisedb.com>
wrote:

I agree the first part is not contentious, so shall we extract this part
of the patch and get that committed for PG15? Or is that too late to
make such changes to the patch?

The minimum viable feature for me, given the written goal for the patch and
the premise 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

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

In particular, I don't see CREATE UNLOGGED SEQUENCE to be all that valuable
since CREATE UNLOGGED TABLE wouldn't leverage it.

The above, possibly only half-baked, patch scope does not change any
existing behavior but allows for the stated goal: an unlogged table having
an unlogged sequence. The DBA just has to execute the ALTER SEQUENCE
command on all relevant sequences. They can't even really get it wrong
since only relevant sequences can be altered. Not having CREATE TABLE make
an unlogged sequence by default is annoying though and likely should be
changed - though it can leverage ALTER SEQUENCE too.

Anything else they wish to do can be done via a combination of ownership
manipulation and, worse case, dropping and recreating the sequence. Though
allowed for unowned unlogged sequences, while outside the explicit goal of
the patch, would be an easy add (just don't error on the ALTER SEQUENCE SET
UNLOGGED when the sequence is unowned).

David J.

On Thu, Mar 31, 2022 at 1:40 PM David G. Johnston <
david.g.johnston@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 ALL SEQUENCES TO UNLOGGED"
command for convenience. Or maybe make it a function - which would allow
for SQL execution against a catalog lookup.

David J.

On 3/31/22 22:40, David G. Johnston wrote:

On Thu, Mar 31, 2022 at 1:05 PM Tomas Vondra
<tomas.vondra@enterprisedb.com <mailto:tomas.vondra@enterprisedb.com>>
wrote:

I agree the first part is not contentious, so shall we extract this part
of the patch and get that committed for PG15? Or is that too late to
make such changes to the patch?

The minimum viable feature for me, given the written goal for the patch
and the premise 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

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

In particular, I don't see CREATE UNLOGGED SEQUENCE to be all that
valuable since CREATE UNLOGGED TABLE wouldn't leverage it.

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 changes persistence for sequences
matching the initial table persistence

IMHO (1) would address vast majority of cases, which simply want the
same persistence for the whole table and all auxiliary objects. (2)
would address use cases requiring different persistence for sequences
(including owned ones).

I'm not sure about (3) though, maybe that's overkill.

Of course, we'll always have problems with older releases, as it's not
clear whether a logged sequence on unlogged table would be desirable or
is used just because unlogged sequences were not supported. (We do have
the same issue for logged tables too, but I doubt anyone really needs
defining unlogged sequences on logged tables.)

So no matter what we do, we'll make the wrong decision in some cases.

The above, possibly only half-baked, patch scope does not change any
existing behavior but allows for the stated goal: an unlogged table
having an unlogged sequence.  The DBA just has to execute the ALTER
SEQUENCE command on all relevant sequences.  They can't even really get
it wrong since only relevant sequences can be altered.  Not having
CREATE TABLE make an unlogged sequence by default is annoying though and
likely should be changed - though it can leverage ALTER SEQUENCE too.

Anything else they wish to do can be done via a combination of ownership
manipulation and, worse case, dropping and recreating the sequence. 
Though allowed for unowned unlogged sequences, while outside the
explicit goal of the patch, would be an easy add (just don't error on
the ALTER SEQUENCE SET UNLOGGED when the sequence is unowned).

Yeah. I think my proposal is pretty close to that, except that the
sequence would first inherit persistence from the table, and there'd be
an ALTER SEQUENCE for owned sequences where it differs. (And non-owned
sequences would be created as logged/unlogged explicitly.)

I don't think we need to worry about old pg_dump versions on new PG
versions, because that's not really supported.

And for old PG versions the behavior would differ a bit depending on the
pg_dump version used. With old pg_dump version, the ALTER SEQUENCE would
not be emitted, so all owned sequences would inherit table persistence.
With new pg_dump we'd get the expected persistence (which might differ).

That's need to be documented, of course.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Mar 31, 2022 at 3:43 PM Tomas Vondra <tomas.vondra@enterprisedb.com>
wrote:

On 3/31/22 22:40, David G. Johnston wrote:

On Thu, Mar 31, 2022 at 1:05 PM Tomas Vondra
<tomas.vondra@enterprisedb.com <mailto:tomas.vondra@enterprisedb.com>>
wrote:

I agree the first part is not contentious, so shall we extract this

part

of the patch and get that committed for PG15? Or is that too late to
make such changes to the patch?

The minimum viable feature for me, given the written goal for the patch
and the premise 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

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

In particular, I don't see CREATE UNLOGGED SEQUENCE to be all that
valuable since CREATE UNLOGGED TABLE wouldn't leverage it.

Hmm, so what about doing a little bit different thing:

1) owned sequences inherit persistence of the table by default

This is the contentious point. If we are going to do it by default - thus
changing existing behavior - I would rather just do it always. This is
also underspecified, there are multiple ways for a sequence to become owned.

Personally I'm for the choice to effectively remove the sequence's own
concept of logged/unlogged when it is owned by a table and to always just
use the table's value.

2) allow ALTER SEQUENCE to change persistence for all sequences (no
restriction for owned sequences)

A generalization that is largely incontrovertible.

3) ALTER TABLE ... SET [UN]LOGGED changes persistence for sequences
matching the initial table persistence

I'm leaning against this, leaving users to set each owned sequence to
logged/unlogged as they see fit if they want something other than
all-or-nothing. I would stick to only providing an easy method to get the
assumed desired all-same behavior.
ALTER TABLE SET [UN]LOGGED, SET ALL SEQUENCES TO [UN]LOGGED;

IMHO (1) would address vast majority of cases, which simply want the
same persistence for the whole table and all auxiliary objects. (2)
would address use cases requiring different persistence for sequences
(including owned ones).

I'm not sure about (3) though, maybe that's overkill.

Of course, we'll always have problems with older releases, as it's not
clear whether a logged sequence on unlogged table would be desirable or
is used just because unlogged sequences were not supported. (We do have
the same issue for logged tables too, but I doubt anyone really needs
defining unlogged sequences on logged tables.)

So no matter what we do, we'll make the wrong decision in some cases.

Again, I don't have too much concern here because you lose very little by
having an unowned sequence. Which is why I'm fine with owned sequences
becoming even moreso implementation details that adhere to the persistence
mode of the owning relation. But if the goal here is to defer such a
decision then the tradeoff is the DBA is given control and they get to
enforce consistency even if they are not benefitting from the flexibility.

Not having
CREATE TABLE make an unlogged sequence by default is annoying though and
likely should be changed - though it can leverage ALTER SEQUENCE too.

Yeah. I think my proposal is pretty close to that, except that the
sequence would first inherit persistence from the table, and there'd be
an ALTER SEQUENCE for owned sequences where it differs. (And non-owned
sequences would be created as logged/unlogged explicitly.)

I don't have any real problem with 1 or 2, they fill out the feature so it
is generally designed as opposed to solving a very specific problem.

For 1:
The "ADD COLUMN" (whether in CREATE TABLE or ALTER TABLE) pathway will
produce a new sequence whose persistence matches that of the target table.
While a behavior change it is one aligned with the goal of the patch for
typical ongoing behavior and should benefit way more people than it may
inconvenience. The "sequence not found" error that would be generated
seems minimally impactful.

The "ALTER SEQUENCE OWNED BY" pathway will not change the sequence's
persistence. This is what pg_dump will use for serial/bigserial
The "ALTER TABLE ALTER COLUMN" pathway will not change the sequence's
persistence. This is what pg_dump will use for generated always as identity

Provide a general purpose ALTER SEQUENCE SET [UN]LOGGED command

Provide an SQL Command to change all owned sequences of a table to be
UNLOGGED or LOGGED (I mentioned a function as well if someone thinks it
worth the time - in lieu of a function a psql script leveraging \gexec may
be nice to reference).

I don't think we need to worry about old pg_dump versions on new PG
versions, because that's not really supported.

Correct.

And for old PG versions the behavior would differ a bit depending on the
pg_dump version used. With old pg_dump version, the ALTER SEQUENCE would
not be emitted,

Correct, nothing else is emitted either...

That's need to be documented, of course.

It (the general promises for pg_dump) is documented.

https://www.postgresql.org/docs/current/app-pgdump.html : Notes

David J.