On 2023-Aug-29, Nathan Bossart wrote:
> On Tue, Aug 29, 2023 at 08:44:02PM +0200, Alvaro Herrera wrote:
> > Makes sense. However, maybe we should replace those ugly defines and
> > their hardcoded values in DefineSequence with a proper array with their
> > names and datatypes.
>
> That might be an improvement, but IIUC you'd still need to enumerate all of
> the columns or data types to make sure you use the right get-Datum
> function. It doesn't help that last_value uses Int64GetDatumFast and
> log_cnt uses Int64GetDatum. I could be missing something, though.
Well, for sure I meant to enumerate everything that was needed,
including the initializer for the value. Like in the attached patch.
However, now that I've actually written it, I don't find it so pretty
anymore, but maybe that's just because I don't know how to write the
array assignment as a single statement instead of a separate statement
for each column.
But this should silence the warnings, anyway.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
commit 50ac445d1e1ad84282d59c09a3937c221d94b968
Author: Alvaro Herrera <alvhe...@alvh.no-ip.org> [Álvaro Herrera <alvhe...@alvh.no-ip.org>]
AuthorDate: Wed Aug 30 16:09:52 2023 +0200
CommitDate: Wed Aug 30 16:41:06 2023 +0200
seq-warnings
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 0b0003fcc8..4798c9cd20 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -129,11 +129,16 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
Relation rel;
HeapTuple tuple;
TupleDesc tupDesc;
- Datum value[SEQ_COL_LASTCOL];
- bool null[SEQ_COL_LASTCOL];
Datum pgs_values[Natts_pg_sequence];
bool pgs_nulls[Natts_pg_sequence];
- int i;
+ struct SequenceColumn
+ {
+ const char *name;
+ Oid type;
+ Datum init_value;
+ } sequence_cols[3];
+ Datum value[3];
+ bool null[3];
/*
* If if_not_exists was given and a relation with the same name already
@@ -166,32 +171,39 @@ DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
&seqform, &seqdataform,
&need_seq_rewrite, &owned_by);
+ sequence_cols[0] = (struct SequenceColumn)
+ {
+ .name = "last_value",
+ .type = INT8OID,
+ .init_value = Int64GetDatumFast(seqdataform.last_value)
+ };
+ sequence_cols[1] = (struct SequenceColumn)
+ {
+ .name = "log_cnt",
+ .type = INT8OID,
+ .init_value = Int64GetDatum((int64) 0)
+ };
+ sequence_cols[2] = (struct SequenceColumn)
+ {
+ .name = "is_called",
+ .type = BOOLOID,
+ .init_value = BoolGetDatum(false)
+ };
+
/*
* Create relation (and fill value[] and null[] for the tuple)
*/
stmt->tableElts = NIL;
- for (i = SEQ_COL_FIRSTCOL; i <= SEQ_COL_LASTCOL; i++)
+ for (int i = 0; i < lengthof(sequence_cols); i++)
{
ColumnDef *coldef;
- switch (i)
- {
- case SEQ_COL_LASTVAL:
- coldef = makeColumnDef("last_value", INT8OID, -1, InvalidOid);
- value[i - 1] = Int64GetDatumFast(seqdataform.last_value);
- break;
- case SEQ_COL_LOG:
- coldef = makeColumnDef("log_cnt", INT8OID, -1, InvalidOid);
- value[i - 1] = Int64GetDatum((int64) 0);
- break;
- case SEQ_COL_CALLED:
- coldef = makeColumnDef("is_called", BOOLOID, -1, InvalidOid);
- value[i - 1] = BoolGetDatum(false);
- break;
- }
-
+ coldef = makeColumnDef(sequence_cols[i].name, sequence_cols[i].type, -1,
+ InvalidOid);
coldef->is_not_null = true;
- null[i - 1] = false;
+
+ value[i] = sequence_cols[i].init_value;
+ null[i] = false;
stmt->tableElts = lappend(stmt->tableElts, coldef);
}
diff --git a/src/include/commands/sequence.h b/src/include/commands/sequence.h
index 7db7b3da7b..784280b26d 100644
--- a/src/include/commands/sequence.h
+++ b/src/include/commands/sequence.h
@@ -31,17 +31,6 @@ typedef struct FormData_pg_sequence_data
typedef FormData_pg_sequence_data *Form_pg_sequence_data;
-/*
- * Columns of a sequence relation
- */
-
-#define SEQ_COL_LASTVAL 1
-#define SEQ_COL_LOG 2
-#define SEQ_COL_CALLED 3
-
-#define SEQ_COL_FIRSTCOL SEQ_COL_LASTVAL
-#define SEQ_COL_LASTCOL SEQ_COL_CALLED
-
/* XLOG stuff */
#define XLOG_SEQ_LOG 0x00
