diff options
Diffstat (limited to 'src/backend')
-rw-r--r-- | src/backend/catalog/dependency.c | 53 | ||||
-rw-r--r-- | src/backend/catalog/heap.c | 28 | ||||
-rw-r--r-- | src/backend/commands/tablecmds.c | 39 |
3 files changed, 88 insertions, 32 deletions
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 6315fc4b2fd..dd0a7d8dac9 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -543,6 +543,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, @@ -550,7 +551,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); @@ -567,6 +571,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: @@ -855,6 +871,16 @@ findDependentObjects(const ObjectAddress *object, 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. */ AcquireDeletionLock(&otherObject, 0); @@ -1588,8 +1614,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, @@ -1602,7 +1630,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; @@ -1626,7 +1654,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; @@ -1657,11 +1686,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 3b8c8b193a7..b8b4768fd69 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -3528,16 +3528,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 16d774fd518..2ab5d91630d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7023,27 +7023,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); @@ -10256,16 +10257,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); |