Re: unlogged sequences

rebased patch, no functional changes

On 11.02.22 10:12, Peter Eisentraut wrote:
> 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.