avoid-alter-sequence-set-logged-v2.patch

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index b37fd688d3..d49d53f35a 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1347,7 +1347,9 @@ init_params(ParseState *pstate, List *options, bool for_identity,
 			/*
 			 * The parser allows this, but it is only for identity columns, in
 			 * which case it is filtered out in parse_utilcmd.c.  We only get
-			 * here if someone puts it into a CREATE SEQUENCE.
+			 * here if someone puts it into a CREATE SEQUENCE.  (The same is
+			 * true for the equally-undocumented LOGGED and UNLOGGED options,
+			 * but for those, the default error below seems sufficient.)
 			 */
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 84cef57a70..4fcab9e3f1 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4908,6 +4908,11 @@ SeqOptElem: AS SimpleTypename
 				{
 					$$ = makeDefElem("increment", (Node *) $3, @1);
 				}
+			| LOGGED
+				{
+					/* not documented, only used by pg_dump */
+					$$ = makeDefElem("logged", NULL, @1);
+				}
 			| MAXVALUE NumericOnly
 				{
 					$$ = makeDefElem("maxvalue", (Node *) $2, @1);
@@ -4945,6 +4950,11 @@ SeqOptElem: AS SimpleTypename
 				{
 					$$ = makeDefElem("restart", (Node *) $3, @1);
 				}
+			| UNLOGGED
+				{
+					/* not documented, only used by pg_dump */
+					$$ = makeDefElem("unlogged", NULL, @1);
+				}
 		;
 
 opt_by:		BY
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 79cad4ab30..2cae834c2c 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -365,30 +365,20 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 {
 	ListCell   *option;
 	DefElem    *nameEl = NULL;
+	DefElem    *loggedEl = NULL;
 	Oid			snamespaceid;
 	char	   *snamespace;
 	char	   *sname;
 	CreateSeqStmt *seqstmt;
 	AlterSeqStmt *altseqstmt;
 	List	   *attnamelist;
-	int			nameEl_idx = -1;
 
 	/* Make a copy of this as we may end up modifying it in the code below */
 	seqoptions = list_copy(seqoptions);
 
 	/*
-	 * Determine namespace and name to use for the sequence.
-	 *
-	 * First, check if a sequence name was passed in as an option.  This is
-	 * used by pg_dump.  Else, generate a name.
-	 *
-	 * Although we use ChooseRelationName, it's not guaranteed that the
-	 * selected sequence name won't conflict; given sufficiently long field
-	 * names, two different serial columns in the same table could be assigned
-	 * the same sequence name, and we'd not notice since we aren't creating
-	 * the sequence quite yet.  In practice this seems quite unlikely to be a
-	 * problem, especially since few people would need two serial columns in
-	 * one table.
+	 * Check for special (undocumented) options used by pg_dump, and remove
+	 * them from the seqoptions list if found.
 	 */
 	foreach(option, seqoptions)
 	{
@@ -399,12 +389,24 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 			if (nameEl)
 				errorConflictingDefElem(defel, cxt->pstate);
 			nameEl = defel;
-			nameEl_idx = foreach_current_index(option);
+			seqoptions = foreach_delete_current(seqoptions, option);
+		}
+		else if (strcmp(defel->defname, "logged") == 0 ||
+				 strcmp(defel->defname, "unlogged") == 0)
+		{
+			if (loggedEl)
+				errorConflictingDefElem(defel, cxt->pstate);
+			loggedEl = defel;
+			seqoptions = foreach_delete_current(seqoptions, option);
 		}
 	}
 
+	/*
+	 * Determine namespace and name to use for the sequence.
+	 */
 	if (nameEl)
 	{
+		/* Use specified name */
 		RangeVar   *rv = makeRangeVarFromNameList(castNode(List, nameEl->arg));
 
 		snamespace = rv->schemaname;
@@ -418,11 +420,20 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 			snamespace = get_namespace_name(snamespaceid);
 		}
 		sname = rv->relname;
-		/* Remove the SEQUENCE NAME item from seqoptions */
-		seqoptions = list_delete_nth_cell(seqoptions, nameEl_idx);
 	}
 	else
 	{
+		/*
+		 * Generate a name.
+		 *
+		 * Although we use ChooseRelationName, it's not guaranteed that the
+		 * selected sequence name won't conflict; given sufficiently long
+		 * field names, two different serial columns in the same table could
+		 * be assigned the same sequence name, and we'd not notice since we
+		 * aren't creating the sequence quite yet.  In practice this seems
+		 * quite unlikely to be a problem, especially since few people would
+		 * need two serial columns in one table.
+		 */
 		if (cxt->rel)
 			snamespaceid = RelationGetNamespace(cxt->rel);
 		else
@@ -452,13 +463,22 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 	seqstmt->sequence = makeRangeVar(snamespace, sname, -1);
 
 	/*
-	 * Copy the persistence of the table.  For CREATE TABLE, we get the
-	 * persistence from cxt->relation, which comes from the CreateStmt in
-	 * progress.  For ALTER TABLE, the parser won't set
+	 * Determine the persistence of the sequence.  If LOGGED or UNLOGGED was
+	 * specified, use that; otherwise copy the persistence of the table.  For
+	 * CREATE TABLE, we get the persistence from cxt->relation, which comes
+	 * from the CreateStmt in progress.  For ALTER TABLE, the parser won't set
 	 * cxt->relation->relpersistence, but we have cxt->rel as the existing
 	 * table, so we copy the persistence from there.
 	 */
-	seqstmt->sequence->relpersistence = cxt->rel ? cxt->rel->rd_rel->relpersistence : cxt->relation->relpersistence;
+	if (loggedEl)
+	{
+		if (strcmp(loggedEl->defname, "logged") == 0)
+			seqstmt->sequence->relpersistence = RELPERSISTENCE_PERMANENT;
+		else
+			seqstmt->sequence->relpersistence = RELPERSISTENCE_UNLOGGED;
+	}
+	else
+		seqstmt->sequence->relpersistence = cxt->rel ? cxt->rel->rd_rel->relpersistence : cxt->relation->relpersistence;
 
 	seqstmt->options = seqoptions;
 
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 546e7e4ce1..d140d6067b 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -17556,6 +17556,15 @@ dumpSequence(Archive *fout, const TableInfo *tbinfo)
 			appendPQExpBufferStr(query, "BY DEFAULT");
 		appendPQExpBuffer(query, " AS IDENTITY (\n    SEQUENCE NAME %s\n",
 						  fmtQualifiedDumpable(tbinfo));
+
+		/*
+		 * Emit persistence option only if it's different from the owning
+		 * table's.  This avoids using this new syntax unnecessarily.
+		 */
+		if (tbinfo->relpersistence != owning_tab->relpersistence)
+			appendPQExpBuffer(query, "    %s\n",
+							  tbinfo->relpersistence == RELPERSISTENCE_UNLOGGED ?
+							  "UNLOGGED" : "LOGGED");
 	}
 	else
 	{
@@ -17588,15 +17597,7 @@ dumpSequence(Archive *fout, const TableInfo *tbinfo)
 					  seq->cache, (seq->cycled ? "\n    CYCLE" : ""));
 
 	if (tbinfo->is_identity_sequence)
-	{
 		appendPQExpBufferStr(query, "\n);\n");
-		if (tbinfo->relpersistence != owning_tab->relpersistence)
-			appendPQExpBuffer(query,
-							  "ALTER SEQUENCE %s SET %s;\n",
-							  fmtQualifiedDumpable(tbinfo),
-							  tbinfo->relpersistence == RELPERSISTENCE_UNLOGGED ?
-							  "UNLOGGED" : "LOGGED");
-	}
 	else
 		appendPQExpBufferStr(query, ";\n");
 
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index 3d554fe327..f14bfccfb1 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -905,3 +905,19 @@ SELECT * FROM itest16;
 
 DROP TABLE itest15;
 DROP TABLE itest16;
+-- For testing of pg_dump and pg_upgrade, leave behind some identity
+-- sequences whose logged-ness doesn't match their owning table's.
+CREATE TABLE identity_dump_logged (a INT GENERATED ALWAYS AS IDENTITY);
+ALTER SEQUENCE identity_dump_logged_a_seq SET UNLOGGED;
+CREATE UNLOGGED TABLE identity_dump_unlogged (a INT GENERATED ALWAYS AS IDENTITY);
+ALTER SEQUENCE identity_dump_unlogged_a_seq SET LOGGED;
+SELECT relname, relpersistence FROM pg_class
+  WHERE relname ~ '^identity_dump_' ORDER BY 1;
+           relname            | relpersistence 
+------------------------------+----------------
+ identity_dump_logged         | p
+ identity_dump_logged_a_seq   | u
+ identity_dump_unlogged       | u
+ identity_dump_unlogged_a_seq | p
+(4 rows)
+
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 84c43a19a3..cb0e05a2f1 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -528,3 +528,12 @@ SELECT * FROM itest15;
 SELECT * FROM itest16;
 DROP TABLE itest15;
 DROP TABLE itest16;
+
+-- For testing of pg_dump and pg_upgrade, leave behind some identity
+-- sequences whose logged-ness doesn't match their owning table's.
+CREATE TABLE identity_dump_logged (a INT GENERATED ALWAYS AS IDENTITY);
+ALTER SEQUENCE identity_dump_logged_a_seq SET UNLOGGED;
+CREATE UNLOGGED TABLE identity_dump_unlogged (a INT GENERATED ALWAYS AS IDENTITY);
+ALTER SEQUENCE identity_dump_unlogged_a_seq SET LOGGED;
+SELECT relname, relpersistence FROM pg_class
+  WHERE relname ~ '^identity_dump_' ORDER BY 1;