Refactor init_params() in sequence.c to not use FormData_pg_sequence_data

init_params() sets up "last_value" and "is_called" for a sequence
relation holdind its metadata, based on the sequence properties in
pg_sequences.  "log_cnt" is the third property that can be updated in
this routine for FormData_pg_sequence_data, tracking when WAL records
should be generated for a sequence after nextval() iterations.  This
routine is called when creating or altering a sequence.

This commit refactors init_params() to not depend anymore on
FormData_pg_sequence_data, removing traces of it in sequence.c, making
easier the manipulation of metadata related to sequences.  The knowledge
about "log_cnt" is replaced with a more general "reset_state" flag, to
let the caller know if the sequence state should be reset.  In the case
of in-core sequences, this relates to WAL logging.  We still need to
depend on FormData_pg_sequence.

Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/ZWlohtKAs0uVVpZ3@paquier.xyz

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 451ae6f7f69401ff80a8c60157cda8363de2cccc..a3c8cff97b03b0af8ad820559c1ce4b47e271f6b 100644 (file)
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -106,7 +106,9 @@ static Form_pg_sequence_data read_seq_tuple(Relation rel,
 static void init_params(ParseState *pstate, List *options, bool for_identity,
                                                bool isInit,
                                                Form_pg_sequence seqform,
-                                               Form_pg_sequence_data seqdataform,
+                                               int64 *last_value,
+                                               bool *reset_state,
+                                               bool *is_called,
                                                bool *need_seq_rewrite,
                                                List **owned_by);
 static void do_setval(Oid relid, int64 next, bool iscalled);
@@ -121,7 +123,9 @@ ObjectAddress
 DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
 {
        FormData_pg_sequence seqform;
-       FormData_pg_sequence_data seqdataform;
+       int64           last_value;
+       bool            reset_state;
+       bool            is_called;
        bool            need_seq_rewrite;
        List       *owned_by;
        CreateStmt *stmt = makeNode(CreateStmt);
@@ -164,7 +168,7 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
 
        /* Check and set all option values */
        init_params(pstate, seq->options, seq->for_identity, true,
-                               &seqform, &seqdataform,
+                               &seqform, &last_value, &reset_state, &is_called,
                                &need_seq_rewrite, &owned_by);
 
        /*
@@ -179,7 +183,7 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
                {
                        case SEQ_COL_LASTVAL:
                                coldef = makeColumnDef("last_value", INT8OID, -1, InvalidOid);
-                               value[i - 1] = Int64GetDatumFast(seqdataform.last_value);
+                               value[i - 1] = Int64GetDatumFast(last_value);
                                break;
                        case SEQ_COL_LOG:
                                coldef = makeColumnDef("log_cnt", INT8OID, -1, InvalidOid);
@@ -448,6 +452,9 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt)
        ObjectAddress address;
        Relation        rel;
        HeapTuple       seqtuple;
+       bool            reset_state = false;
+       bool            is_called;
+       int64           last_value;
        HeapTuple       newdatatuple;
 
        /* Open and lock sequence, and check for ownership along the way. */
@@ -481,12 +488,14 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt)
        /* copy the existing sequence data tuple, so it can be modified locally */
        newdatatuple = heap_copytuple(&datatuple);
        newdataform = (Form_pg_sequence_data) GETSTRUCT(newdatatuple);
+       last_value = newdataform->last_value;
+       is_called = newdataform->is_called;
 
        UnlockReleaseBuffer(buf);
 
        /* Check and set new values */
        init_params(pstate, stmt->options, stmt->for_identity, false,
-                               seqform, newdataform,
+                               seqform, &last_value, &reset_state, &is_called,
                                &need_seq_rewrite, &owned_by);
 
        /* If needed, rewrite the sequence relation itself */
@@ -513,6 +522,10 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt)
                /*
                 * Insert the modified tuple into the new storage file.
                 */
+               newdataform->last_value = last_value;
+               newdataform->is_called = is_called;
+               if (reset_state)
+                       newdataform->log_cnt = 0;
                fill_seq_with_data(seqrel, newdatatuple);
        }
 
@@ -1236,17 +1249,19 @@ read_seq_tuple(Relation rel, Buffer *buf, HeapTuple seqdatatuple)
 /*
  * init_params: process the options list of CREATE or ALTER SEQUENCE, and
  * store the values into appropriate fields of seqform, for changes that go
- * into the pg_sequence catalog, and fields of seqdataform for changes to the
- * sequence relation itself.  Set *need_seq_rewrite to true if we changed any
- * parameters that require rewriting the sequence's relation (interesting for
- * ALTER SEQUENCE).  Also set *owned_by to any OWNED BY option, or to NIL if
- * there is none.
+ * into the pg_sequence catalog, and fields for changes to the sequence
+ * relation itself (*is_called, *last_value and *reset_state).  Set
+ * *need_seq_rewrite to true if we changed any parameters that require
+ * rewriting the sequence's relation (interesting for ALTER SEQUENCE).  Also
+ * set *owned_by to any OWNED BY option, or to NIL if there is none.  Set
+ * *reset_state to true if the internal state of the sequence needs to be
+ * reset, affecting future nextval() calls, for example with WAL logging.
  *
  * If isInit is true, fill any unspecified options with default values;
  * otherwise, do not change existing options that aren't explicitly overridden.
  *
  * Note: we force a sequence rewrite whenever we change parameters that affect
- * generation of future sequence values, even if the seqdataform per se is not
+ * generation of future sequence values, even if the metadata per se is not
  * changed.  This allows ALTER SEQUENCE to behave transactionally.  Currently,
  * the only option that doesn't cause that is OWNED BY.  It's *necessary* for
  * ALTER SEQUENCE OWNED BY to not rewrite the sequence, because that would
@@ -1257,7 +1272,9 @@ static void
 init_params(ParseState *pstate, List *options, bool for_identity,
                        bool isInit,
                        Form_pg_sequence seqform,
-                       Form_pg_sequence_data seqdataform,
+                       int64 *last_value,
+                       bool *reset_state,
+                       bool *is_called,
                        bool *need_seq_rewrite,
                        List **owned_by)
 {
@@ -1363,11 +1380,11 @@ init_params(ParseState *pstate, List *options, bool for_identity,
        }
 
        /*
-        * We must reset log_cnt when isInit or when changing any parameters that
-        * would affect future nextval allocations.
+        * We must reset the state of the sequence when isInit or when changing
+        * any parameters that would affect future nextval allocations.
         */
        if (isInit)
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
 
        /* AS type */
        if (as_type != NULL)
@@ -1416,7 +1433,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
                        ereport(ERROR,
                                        (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                         errmsg("INCREMENT must not be zero")));
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
        }
        else if (isInit)
        {
@@ -1428,7 +1445,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
        {
                seqform->seqcycle = boolVal(is_cycled->arg);
                Assert(BoolIsValid(seqform->seqcycle));
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
        }
        else if (isInit)
        {
@@ -1439,7 +1456,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
        if (max_value != NULL && max_value->arg)
        {
                seqform->seqmax = defGetInt64(max_value);
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
        }
        else if (isInit || max_value != NULL || reset_max_value)
        {
@@ -1455,7 +1472,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
                }
                else
                        seqform->seqmax = -1;   /* descending seq */
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
        }
 
        /* Validate maximum value.  No need to check INT8 as seqmax is an int64 */
@@ -1471,7 +1488,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
        if (min_value != NULL && min_value->arg)
        {
                seqform->seqmin = defGetInt64(min_value);
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
        }
        else if (isInit || min_value != NULL || reset_min_value)
        {
@@ -1487,7 +1504,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
                }
                else
                        seqform->seqmin = 1;    /* ascending seq */
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
        }
 
        /* Validate minimum value.  No need to check INT8 as seqmin is an int64 */
@@ -1538,30 +1555,30 @@ init_params(ParseState *pstate, List *options, bool for_identity,
        if (restart_value != NULL)
        {
                if (restart_value->arg != NULL)
-                       seqdataform->last_value = defGetInt64(restart_value);
+                       *last_value = defGetInt64(restart_value);
                else
-                       seqdataform->last_value = seqform->seqstart;
-               seqdataform->is_called = false;
-               seqdataform->log_cnt = 0;
+                       *last_value = seqform->seqstart;
+               *is_called = false;
+               *reset_state = true;
        }
        else if (isInit)
        {
-               seqdataform->last_value = seqform->seqstart;
-               seqdataform->is_called = false;
+               *last_value = seqform->seqstart;
+               *is_called = false;
        }
 
        /* crosscheck RESTART (or current value, if changing MIN/MAX) */
-       if (seqdataform->last_value < seqform->seqmin)
+       if (*last_value < seqform->seqmin)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                 errmsg("RESTART value (%" PRId64 ") cannot be less than MINVALUE (%" PRId64 ")",
-                                               seqdataform->last_value,
+                                               *last_value,
                                                seqform->seqmin)));
-       if (seqdataform->last_value > seqform->seqmax)
+       if (*last_value > seqform->seqmax)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                 errmsg("RESTART value (%" PRId64 ") cannot be greater than MAXVALUE (%" PRId64 ")",
-                                               seqdataform->last_value,
+                                               *last_value,
                                                seqform->seqmax)));
 
        /* CACHE */
@@ -1573,7 +1590,7 @@ init_params(ParseState *pstate, List *options, bool for_identity,
                                        (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                         errmsg("CACHE (%" PRId64 ") must be greater than zero",
                                                        seqform->seqcache)));
-               seqdataform->log_cnt = 0;
+               *reset_state = true;
        }
        else if (isInit)
        {