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

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 =

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