diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 4f1d3653575b4fc9e6c849b39f0e360f69445dbe..a03347648308e785edf81269e3db5ec0a91e3723 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -523,6 +523,7 @@ findDependentObjects(const ObjectAddress *object,
ObjectIdGetDatum(object->objectId));
if (object->objectSubId != 0)
{
+ /* Consider only dependencies of this sub-object */
ScanKeyInit(&key[2],
Anum_pg_depend_objsubid,
BTEqualStrategyNumber, F_INT4EQ,
@@ -530,7 +531,10 @@ findDependentObjects(const ObjectAddress *object,
nkeys = 3;
}
else
+ {
+ /* Consider dependencies of this object and any sub-objects it has */
nkeys = 2;
+ }
scan = systable_beginscan(*depRel, DependDependerIndexId, true,
NULL, nkeys, key);
@@ -543,6 +547,18 @@ findDependentObjects(const ObjectAddress *object,
otherObject.objectId = foundDep->refobjid;
otherObject.objectSubId = foundDep->refobjsubid;
+ /*
+ * When scanning dependencies of a whole object, we may find rows
+ * linking sub-objects of the object to the object itself. (Normally,
+ * such a dependency is implicit, but we must make explicit ones in
+ * some cases involving partitioning.) We must ignore such rows to
+ * avoid infinite recursion.
+ */
+ if (otherObject.classId == object->classId &&
+ otherObject.objectId == object->objectId &&
+ object->objectSubId == 0)
+ continue;
+
switch (foundDep->deptype)
{
case DEPENDENCY_NORMAL:
@@ -739,6 +755,16 @@ findDependentObjects(const ObjectAddress *object,
otherObject.objectId = foundDep->objid;
otherObject.objectSubId = foundDep->objsubid;
+ /*
+ * If what we found is a sub-object of the current object, just ignore
+ * it. (Normally, such a dependency is implicit, but we must make
+ * explicit ones in some cases involving partitioning.)
+ */
+ if (otherObject.classId == object->classId &&
+ otherObject.objectId == object->objectId &&
+ object->objectSubId == 0)
+ continue;
+
/*
* Must lock the dependent object before recursing to it.
*/
@@ -1388,8 +1414,10 @@ recordDependencyOnExpr(const ObjectAddress *depender,
* As above, but only one relation is expected to be referenced (with
* varno = 1 and varlevelsup = 0). Pass the relation OID instead of a
* range table. An additional frammish is that dependencies on that
- * relation (or its component columns) will be marked with 'self_behavior',
- * whereas 'behavior' is used for everything else.
+ * relation's component columns will be marked with 'self_behavior',
+ * whereas 'behavior' is used for everything else; also, if 'reverse_self'
+ * is true, those dependencies are reversed so that the columns are made
+ * to depend on the table not vice versa.
*
* NOTE: the caller should ensure that a whole-table dependency on the
* specified relation is created separately, if one is needed. In particular,
@@ -1402,7 +1430,7 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
Node *expr, Oid relId,
DependencyType behavior,
DependencyType self_behavior,
- bool ignore_self)
+ bool reverse_self)
{
find_expr_references_context context;
RangeTblEntry rte;
@@ -1425,7 +1453,8 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
eliminate_duplicate_dependencies(context.addrs);
/* Separate self-dependencies if necessary */
- if (behavior != self_behavior && context.addrs->numrefs > 0)
+ if ((behavior != self_behavior || reverse_self) &&
+ context.addrs->numrefs > 0)
{
ObjectAddresses *self_addrs;
ObjectAddress *outobj;
@@ -1456,11 +1485,23 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
}
context.addrs->numrefs = outrefs;
- /* Record the self-dependencies */
- if (!ignore_self)
+ /* Record the self-dependencies with the appropriate direction */
+ if (!reverse_self)
recordMultipleDependencies(depender,
self_addrs->refs, self_addrs->numrefs,
self_behavior);
+ else
+ {
+ /* Can't use recordMultipleDependencies, so do it the hard way */
+ int selfref;
+
+ for (selfref = 0; selfref < self_addrs->numrefs; selfref++)
+ {
+ ObjectAddress *thisobj = self_addrs->refs + selfref;
+
+ recordDependencyOn(thisobj, depender, self_behavior);
+ }
+ }
free_object_addresses(self_addrs);
}
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 415194e0193f5068a3375d54f283920c2e08b878..9e7b1186ee9b3ee6f48344e2fd4ad7fa2bced2ed 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3444,16 +3444,36 @@ StorePartitionKey(Relation rel,
}
/*
- * Anything mentioned in the expressions. We must ignore the column
- * references, which will depend on the table itself; there is no separate
- * partition key object.
+ * The partitioning columns are made internally dependent on the table,
+ * because we cannot drop any of them without dropping the whole table.
+ * (ATExecDropColumn independently enforces that, but it's not bulletproof
+ * so we need the dependencies too.)
+ */
+ for (i = 0; i < partnatts; i++)
+ {
+ if (partattrs[i] == 0)
+ continue; /* ignore expressions here */
+
+ referenced.classId = RelationRelationId;
+ referenced.objectId = RelationGetRelid(rel);
+ referenced.objectSubId = partattrs[i];
+
+ recordDependencyOn(&referenced, &myself, DEPENDENCY_INTERNAL);
+ }
+
+ /*
+ * Also consider anything mentioned in partition expressions. External
+ * references (e.g. functions) get NORMAL dependencies. Table columns
+ * mentioned in the expressions are handled the same as plain partitioning
+ * columns, i.e. they become internally dependent on the whole table.
*/
if (partexprs)
recordDependencyOnSingleRelExpr(&myself,
(Node *) partexprs,
RelationGetRelid(rel),
DEPENDENCY_NORMAL,
- DEPENDENCY_AUTO, true);
+ DEPENDENCY_INTERNAL,
+ true /* reverse the self-deps */ );
/*
* We must invalidate the relcache so that the next
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7f3e8bd79132c0083b50638ec6600c0215a4b341..2ceda4f0a16547a4dda113cd683cef7d8621a84b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6825,27 +6825,28 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
errmsg("cannot drop system column \"%s\"",
colName)));
- /* Don't drop inherited columns */
+ /*
+ * Don't drop inherited columns, unless recursing (presumably from a drop
+ * of the parent column)
+ */
if (targetatt->attinhcount > 0 && !recursing)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("cannot drop inherited column \"%s\"",
colName)));
- /* Don't drop columns used in the partition key */
+ /*
+ * Don't drop columns used in the partition key, either. (If we let this
+ * go through, the key column's dependencies would cause a cascaded drop
+ * of the whole table, which is surely not what the user expected.)
+ */
if (has_partition_attrs(rel,
bms_make_singleton(attnum - FirstLowInvalidHeapAttributeNumber),
&is_expr))
- {
- if (!is_expr)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot drop column named in partition key")));
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot drop column referenced in partition key expression")));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot drop column \"%s\" because it is part of the partition key of relation \"%s\"",
+ colName, RelationGetRelationName(rel))));
ReleaseSysCache(tuple);
@@ -9554,16 +9555,10 @@ ATPrepAlterColumnType(List **wqueue,
if (has_partition_attrs(rel,
bms_make_singleton(attnum - FirstLowInvalidHeapAttributeNumber),
&is_expr))
- {
- if (!is_expr)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot alter type of column named in partition key")));
- else
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot alter type of column referenced in partition key expression")));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot alter column \"%s\" because it is part of the partition key of relation \"%s\"",
+ colName, RelationGetRelationName(rel))));
/* Look up the target type */
typenameTypeIdAndMod(NULL, typeName, &targettype, &targettypmod);
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index c454c1e508bef35bb79848b062ae4e3098829a92..617b7cd35d9a8cae1b9d03970e838f253bdf63d5 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -1202,6 +1202,24 @@ repairDependencyLoop(DumpableObject **loop,
}
}
+ /*
+ * Loop of table with itself --- just ignore it.
+ *
+ * (Actually, what this arises from is a dependency of a table column on
+ * another column, which happens with generated columns; or a dependency
+ * of a table column on the whole table, which happens with partitioning.
+ * But we didn't pay attention to sub-object IDs while collecting the
+ * dependency data, so we can't see that here.)
+ */
+ if (nLoop == 1)
+ {
+ if (loop[0]->objType == DO_TABLE)
+ {
+ removeObjectDependency(loop[0], loop[0]->dumpId);
+ return;
+ }
+ }
+
/*
* If all the objects are TABLE_DATA items, what we must have is a
* circular set of foreign key constraints (or a single self-referential
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 46c271a46c67fb0203cbb21b158aa74667482e95..184215a367d95a0066b5664325a9aef5455d9c0c 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -209,7 +209,7 @@ extern void recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
Node *expr, Oid relId,
DependencyType behavior,
DependencyType self_behavior,
- bool ignore_self);
+ bool reverse_self);
extern ObjectClass getObjectClass(const ObjectAddress *object);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 7c0f48d8ed4c3d84b96100fc8d60a0f2ebb8f1ad..44c178ff4af7e379ab8639f4351d485d0f7ea12a 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3538,13 +3538,13 @@ LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
^
-- cannot drop column that is part of the partition key
ALTER TABLE partitioned DROP COLUMN a;
-ERROR: cannot drop column named in partition key
+ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned ALTER COLUMN a TYPE char(5);
-ERROR: cannot alter type of column named in partition key
+ERROR: cannot alter column "a" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned DROP COLUMN b;
-ERROR: cannot drop column referenced in partition key expression
+ERROR: cannot drop column "b" because it is part of the partition key of relation "partitioned"
ALTER TABLE partitioned ALTER COLUMN b TYPE char(5);
-ERROR: cannot alter type of column referenced in partition key expression
+ERROR: cannot alter column "b" because it is part of the partition key of relation "partitioned"
-- partitioned table cannot participate in regular inheritance
CREATE TABLE nonpartitioned (
a int,
@@ -4046,9 +4046,9 @@ ERROR: cannot change inheritance of a partition
-- partitioned tables; for example, part_5, which is list_parted2's
-- partition, is partitioned on b;
ALTER TABLE list_parted2 DROP COLUMN b;
-ERROR: cannot drop column named in partition key
+ERROR: cannot drop column "b" because it is part of the partition key of relation "part_5"
ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
-ERROR: cannot alter type of column named in partition key
+ERROR: cannot alter column "b" because it is part of the partition key of relation "part_5"
-- dropping non-partition key columns should be allowed on the parent table.
ALTER TABLE list_parted DROP COLUMN b;
SELECT * FROM list_parted;
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index d2ee3f4035a07853ba3448deae725b3958acb336..c25117e47c7ef327214bc56b46e3124f7e71ea0a 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -456,6 +456,42 @@ Partition of: partitioned2 FOR VALUES FROM ('-1', 'aaaaa') TO (100, 'ccccc')
Partition constraint: (((a + 1) IS NOT NULL) AND (substr(b, 1, 5) IS NOT NULL) AND (((a + 1) > '-1'::integer) OR (((a + 1) = '-1'::integer) AND (substr(b, 1, 5) >= 'aaaaa'::text))) AND (((a + 1) < 100) OR (((a + 1) = 100) AND (substr(b, 1, 5) < 'ccccc'::text))))
DROP TABLE partitioned, partitioned2;
+-- check that dependencies of partition columns are handled correctly
+create domain intdom1 as int;
+create table partitioned (
+ a intdom1,
+ b text
+) partition by range (a);
+alter table partitioned drop column a; -- fail
+ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
+drop domain intdom1; -- fail, requires cascade
+ERROR: cannot drop type intdom1 because other objects depend on it
+DETAIL: table partitioned depends on type intdom1
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+drop domain intdom1 cascade;
+NOTICE: drop cascades to table partitioned
+table partitioned; -- gone
+ERROR: relation "partitioned" does not exist
+LINE 1: table partitioned;
+ ^
+-- likewise for columns used in partition expressions
+create domain intdom1 as int;
+create table partitioned (
+ a intdom1,
+ b text
+) partition by range (plusone(a));
+alter table partitioned drop column a; -- fail
+ERROR: cannot drop column "a" because it is part of the partition key of relation "partitioned"
+drop domain intdom1; -- fail, requires cascade
+ERROR: cannot drop type intdom1 because other objects depend on it
+DETAIL: table partitioned depends on type intdom1
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+drop domain intdom1 cascade;
+NOTICE: drop cascades to table partitioned
+table partitioned; -- gone
+ERROR: relation "partitioned" does not exist
+LINE 1: table partitioned;
+ ^
--
-- Partitions
--
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index f61cd595f765b6dd5759ff4f51732daa1ca2e7e8..27e6f59e87ce80f218c07db15f4390bfce7f746a 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -431,6 +431,39 @@ CREATE TABLE part2_1 PARTITION OF partitioned2 FOR VALUES FROM (-1, 'aaaaa') TO
DROP TABLE partitioned, partitioned2;
+-- check that dependencies of partition columns are handled correctly
+create domain intdom1 as int;
+
+create table partitioned (
+ a intdom1,
+ b text
+) partition by range (a);
+
+alter table partitioned drop column a; -- fail
+
+drop domain intdom1; -- fail, requires cascade
+
+drop domain intdom1 cascade;
+
+table partitioned; -- gone
+
+-- likewise for columns used in partition expressions
+create domain intdom1 as int;
+
+create table partitioned (
+ a intdom1,
+ b text
+) partition by range (plusone(a));
+
+alter table partitioned drop column a; -- fail
+
+drop domain intdom1; -- fail, requires cascade
+
+drop domain intdom1 cascade;
+
+table partitioned; -- gone
+
+
--
-- Partitions
--