Horton

Thread

Commits

  1. Avoid invalid array reference in transformAlterTableStmt().

  2. Fix second race condition in 002_archiving.pl with archive_cleanup_command

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

    The Post Office <noreply@postgresql.org> — 2022-04-18T11:13:28Z

    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 =

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-18T15:36:51Z

    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