BUG #17465: Wrong error message while trying to change system column type.

Started by PG Bug reporting formalmost 4 years ago2 messagesbugs
Jump to latest
#1PG Bug reporting form
noreply@postgresql.org

The following bug has been logged on the website:

Bug reference: 17465
Logged by: Roman Zharkov
Email address: r.zharkov@postgrespro.ru
PostgreSQL version: 13.4
Operating system: Windows
Description:

Hello,
accidentally I tried to change a system column data type on Windows and got
a strange error message "no owned sequence found":

psql (15devel)
postgres=# alter table t1 alter column cmin type text;
ERROR: no owned sequence found
postgres=# alter table t1 alter column cmax type text;
ERROR: cannot alter system column "cmax"

This happens due incorrect checking for negative attnum:
if (TupleDescAttr(tupdesc, attnum - 1)->attidentity)
( line
https://github.com/postgres/postgres/blob/1a8b110539efe18803c1fa8aa452a2178dbad9a9/src/backend/parser/parse_utilcmd.c#L3427
)

I think the ATParseTransformCmd() function needs the same check as the check
in the ATPrepAlterColumnType() function.

diff --git a/src/backend/commands/tablecmds.c
b/src/backend/commands/tablecmds.c
index 2cd8546d471..0034a06ea71 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4630,10 +4630,11 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd
*cmd,
 		case AT_AlterColumnType:	/* ALTER COLUMN TYPE */
 			ATSimplePermissions(cmd->subtype, rel,
 								ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE);
+
+			Assert(cmd != NULL);
 			/* See comments for ATPrepAlterColumnType */
 			cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, recurse, lockmode,
 									  AT_PASS_UNSET, context);
-			Assert(cmd != NULL);
 			/* Performs own recursion */
 			ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd,
 								  lockmode, context);
@@ -5250,6 +5251,26 @@ ATParseTransformCmd(List **wqueue, AlteredTableInfo
*tab, Relation rel,
 	List	   *beforeStmts;
 	List	   *afterStmts;
 	ListCell   *lc;
+	char *colName = cmd->name;
+	HeapTuple	tuple;
+	Form_pg_attribute attTup;
+	AttrNumber	attnum;
+
+	tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
+	if (!HeapTupleIsValid(tuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_COLUMN),
+				 errmsg("column \"%s\" of relation \"%s\" does not exist",
+						colName, RelationGetRelationName(rel))));
+	attTup = (Form_pg_attribute)GETSTRUCT(tuple);
+	attnum = attTup->attnum;
+
+	/* Can't alter a system attribute */
+	if (attnum <= 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot alter system column \"%s\"",
+						colName)));

/* Gin up an AlterTableStmt with just this subcommand and this table */
atstmt->relation =

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: PG Bug reporting form (#1)
Re: BUG #17465: Wrong error message while trying to change system column type.

PG Bug reporting form <noreply@postgresql.org> writes:

accidentally I tried to change a system column data type on Windows and got
a strange error message "no owned sequence found":

postgres=# alter table t1 alter column cmin type text;
ERROR: no owned sequence found
postgres=# alter table t1 alter column cmax type text;
ERROR: cannot alter system column "cmax"

Yeah, same here.

I think the ATParseTransformCmd() function needs the same check as the check
in the ATPrepAlterColumnType() function.

That seems pretty duplicative. I think it's sufficient to do something
like

-                    if (TupleDescAttr(tupdesc, attnum - 1)->attidentity)
+                    if (attnum > 0 &&
+                        TupleDescAttr(tupdesc, attnum - 1)->attidentity)

in the faulty code, and let the actual error message come out where
it does now.

Will fix, thanks for the report!

regards, tom lane