Identity columns don't work if they own more than one sequence.

So if one tries to convert a "serial" column to an identity column,
the following can happen:

test=> CREATE TABLE ser(id serial);
CREATE TABLE
test=> ALTER TABLE ser ALTER id ADD GENERATED ALWAYS AS IDENTITY;
ERROR:  column "id" of relation "ser" already has a default value

Hm, ok, let's drop the column default value.

test=> ALTER TABLE ser ALTER id DROP DEFAULT;
ALTER TABLE

Now it works:

test=> ALTER TABLE ser ALTER id ADD GENERATED ALWAYS AS IDENTITY;
ALTER TABLE

But not very much:

test=> INSERT INTO ser (id) VALUES (DEFAULT);
ERROR:  more than one owned sequence found


I propose that we check if there already is a dependent sequence
before adding an identity column.

The attached patch does that, and also forbids setting the ownership
of a sequence to an identity column.

I think this should be backpatched.

Yours,
Laurenz Albe
From ab536da87fa8ffc70469d3dbdaf3e1b84b0ef793 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Sun, 14 Apr 2019 17:37:03 +0200
Subject: [PATCH] Make sure identity columns own only a single sequence

If an identity column owns more than one sequence, inserting the
default value will fail with "more than one owned sequence found".

Consequently, we should prevent this from happening, and disallow
a) turning a column that already owns a sequence into an identity column
b) setting ownership of a sequence to an identity column
---
 src/backend/commands/sequence.c  | 20 +++++++++++++++++---
 src/backend/commands/tablecmds.c | 11 +++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index e9add1b987..dc3bff5fdf 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1644,6 +1644,7 @@ process_owned_by(Relation seqrel, List *owned_by, bool for_identity)
 	int			nnames;
 	Relation	tablerel;
 	AttrNumber	attnum;
+	HeapTuple	heaptup;
 
 	deptype = for_identity ? DEPENDENCY_INTERNAL : DEPENDENCY_AUTO;
 
@@ -1694,9 +1695,22 @@ process_owned_by(Relation seqrel, List *owned_by, bool for_identity)
 					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 					 errmsg("sequence must be in same schema as table it is linked to")));
 
-		/* Now, fetch the attribute number from the system cache */
-		attnum = get_attnum(RelationGetRelid(tablerel), attrname);
-		if (attnum == InvalidAttrNumber)
+		/* Now, fetch the attribute from the system cache */
+		heaptup = SearchSysCacheAttName(RelationGetRelid(tablerel), attrname);
+		if (HeapTupleIsValid(heaptup))
+		{
+			Form_pg_attribute att_tup = (Form_pg_attribute) GETSTRUCT(heaptup);
+			bool	is_identity = att_tup->attidentity;
+
+			attnum = att_tup->attnum;
+			ReleaseSysCache(heaptup);
+			if (is_identity && !for_identity)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("column \"%s\" of relation \"%s\" is an identity column",
+								attrname, RelationGetRelationName(tablerel))));
+		}
+		else
 			ereport(ERROR,
 					(errcode(ERRCODE_UNDEFINED_COLUMN),
 					 errmsg("column \"%s\" of relation \"%s\" does not exist",
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d48a947f7c..9ea9dae2fb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6378,6 +6378,17 @@ ATExecAddIdentity(Relation rel, const char *colName,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("column \"%s\" of relation \"%s\" already has a default value",
 						colName, RelationGetRelationName(rel))));
+	/*
+	 * Make sure that the column does not already own a sequence.
+	 * Otherwise, inserting a default value would fail, since it is not
+	 * clear which sequence should be used.
+	 */
+	if (getOwnedSequences(RelationGetRelid(rel), attnum) != NIL)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("column \"%s\" of relation \"%s\" already owns a sequence",
+						colName, RelationGetRelationName(rel)),
+				 errhint("Drop the dependent sequence before making the column an identity column.")));
 
 	attTup->attidentity = cdef->identity;
 	CatalogTupleUpdate(attrelation, &tuple->t_self, tuple);
-- 
2.20.1

Reply via email to