From 65377e0b9c0e0397b1598b38b6a7fb8b6f740d39 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Sun, 6 Feb 2011 00:26:27 -0500 Subject: [PATCH] Tighten ALTER FOREIGN TABLE .. SET DATA TYPE checks. If the foreign table's rowtype is being used as the type of a column in another table, we can't just up and change its data type. This was already checked for composite types and ordinary tables, but we previously failed to enforce it for foreign tables. --- src/backend/commands/tablecmds.c | 43 +++++++++++----------- src/backend/commands/typecmds.c | 2 +- src/include/commands/tablecmds.h | 3 +- src/test/regress/expected/foreign_data.out | 7 +++- src/test/regress/sql/foreign_data.sql | 6 ++- 5 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 7a02899121..6a17399eb6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3391,8 +3391,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) */ if (newrel) find_composite_type_dependencies(oldrel->rd_rel->reltype, - RelationGetRelationName(oldrel), - NULL); + oldrel->rd_rel->relkind, + RelationGetRelationName(oldrel)); /* * Generate the constraint and default execution states @@ -3860,9 +3860,8 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, * to reject the ALTER. (How safe is this really?) */ void -find_composite_type_dependencies(Oid typeOid, - const char *origTblName, - const char *origTypeName) +find_composite_type_dependencies(Oid typeOid, char origRelkind, + const char *origRelname) { Relation depRel; ScanKeyData key[2]; @@ -3905,20 +3904,19 @@ find_composite_type_dependencies(Oid typeOid, if (rel->rd_rel->relkind == RELKIND_RELATION) { - if (origTblName) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter table \"%s\" because column \"%s\".\"%s\" uses its rowtype", - origTblName, - RelationGetRelationName(rel), - NameStr(att->attname)))); + const char *msg; + if (origRelkind == RELKIND_COMPOSITE_TYPE) + msg = gettext_noop("cannot alter type \"%s\" because column \"%s\".\"%s\" uses it"); + else if (origRelkind == RELKIND_FOREIGN_TABLE) + msg = gettext_noop("cannot alter foreign table \"%s\" because column \"%s\".\"%s\" uses its rowtype"); else - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter type \"%s\" because column \"%s\".\"%s\" uses it", - origTypeName, - RelationGetRelationName(rel), - NameStr(att->attname)))); + msg = gettext_noop("cannot alter table \"%s\" because column \"%s\".\"%s\" uses its rowtype"); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg(msg, + origRelname, + RelationGetRelationName(rel), + NameStr(att->attname)))); } else if (OidIsValid(rel->rd_rel->reltype)) { @@ -3927,7 +3925,7 @@ find_composite_type_dependencies(Oid typeOid, * recursively check for indirect dependencies via its rowtype. */ find_composite_type_dependencies(rel->rd_rel->reltype, - origTblName, origTypeName); + origRelkind, origRelname); } relation_close(rel, AccessShareLock); @@ -3943,7 +3941,7 @@ find_composite_type_dependencies(Oid typeOid, */ arrayOid = get_array_type(typeOid); if (OidIsValid(arrayOid)) - find_composite_type_dependencies(arrayOid, origTblName, origTypeName); + find_composite_type_dependencies(arrayOid, origRelkind, origRelname); } @@ -6444,14 +6442,15 @@ ATPrepAlterColumnType(List **wqueue, errmsg("ALTER TYPE USING is not supported on foreign tables"))); } - if (tab->relkind == RELKIND_COMPOSITE_TYPE) + if (tab->relkind == RELKIND_COMPOSITE_TYPE + || tab->relkind == RELKIND_FOREIGN_TABLE) { /* * For composite types, do this check now. Tables will check * it later when the table is being rewritten. */ find_composite_type_dependencies(rel->rd_rel->reltype, - NULL, + rel->rd_rel->relkind, RelationGetRelationName(rel)); } diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 2ecb238b9e..619929ffe5 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -2183,7 +2183,7 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) */ if (OidIsValid(rel->rd_rel->reltype)) find_composite_type_dependencies(rel->rd_rel->reltype, - NULL, + RELKIND_COMPOSITE_TYPE, format_type_be(domainOid)); /* Otherwise we can ignore views, composite types, etc */ diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index d3deffb3b8..b266230778 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -53,8 +53,7 @@ extern void RenameRelationInternal(Oid myrelid, Oid namespaceId); extern void find_composite_type_dependencies(Oid typeOid, - const char *origTblName, - const char *origTypeName); + char origRelkind, const char *origRelname); extern AttrNumber *varattnos_map(TupleDesc olddesc, TupleDesc newdesc); extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema); diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index 6f2a7a6efd..d6c650be14 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -692,7 +692,12 @@ ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10) using '0'; -- ERROR ERROR: ALTER TYPE USING is not supported on foreign tables ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10); ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE text; -ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); +-- can't change the column type if it's used elsewhere +CREATE TABLE use_ft1_column_type (x ft1); +ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer; -- ERROR +ERROR: cannot alter foreign table "ft1" because column "use_ft1_column_type"."x" uses its rowtype +DROP TABLE use_ft1_column_type; +ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); -- ERROR ERROR: "ft1" is not a table ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const; -- ERROR ERROR: "ft1" is not a table diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql index 655ddc0d0c..86b698a1b6 100644 --- a/src/test/regress/sql/foreign_data.sql +++ b/src/test/regress/sql/foreign_data.sql @@ -293,7 +293,11 @@ ALTER FOREIGN TABLE ft1 ALTER COLUMN c7 DROP NOT NULL; ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10) using '0'; -- ERROR ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE char(10); ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE text; -ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); +-- can't change the column type if it's used elsewhere +CREATE TABLE use_ft1_column_type (x ft1); +ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer; -- ERROR +DROP TABLE use_ft1_column_type; +ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); -- ERROR ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const; -- ERROR ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const; ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c1_check; -- GitLab