Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages) - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages) |
| Date | |
| Msg-id | 18885.1549642539@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages) (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: Fixing findDependentObjects()'s dependency on scan order(regressions in DROP diagnostic messages)
Re: Fixing findDependentObjects()'s dependency on scan order(regressions in DROP diagnostic messages) |
| List | pgsql-hackers |
I wrote:
> I believe that we need to establish the following principles:
>
> * The terminology "internal_auto" is disastrously unhelpful.
> I propose calling these things "partition" dependencies instead.
>
> * Partition dependencies are not singletons. They should *always*
> come in pairs, one on the parent partitioned object (partitioned
> index, constraint, trigger, etc) and one on the child partitioned
> table. (Have I mentioned that our partition/partitioned terminology
> is also a disaster?) Maybe someday there'll be a reason to have
> three or more of these for the same dependent object, but there's
> no reason to have just one --- you should use an internal dependency
> instead for that case.
>
> * Partition dependencies are made *in addition to*, not instead of,
> the dependencies the object would normally have. In this way,
> for example, the unlink action in IndexSetParentIndex would just
> delete the partition dependencies and not have to worry about putting
> back the index's usual dependencies. Consistent with that, it's
> simply wrong that index_create sometimes marks the "usual"
> dependencies as internal_auto rather than auto. (It wasn't even
> doing that consistently; expression and predicate column dependencies
> were still "auto", which makes no sense at all.) They should go
> back to being plain auto with the partition dependencies being added
> separately. That will work properly given the changes that say that
> arriving at a partition-dependent object via an auto dependency is
> not sufficient license to delete the object.
Here's a patch along these lines. Some notes worth making:
* After spending a fair amount of effort on the description of the
dependency types in catalogs.sgml, I decided to just rip out the
duplicative text in dependency.h in favor of a pointer to catalogs.sgml.
If anybody's really hot to have two copies, we could put that back, but
I don't see the point.
* The core idea of the changes in dependency.c is just to wait till the
end of the entire dependency tree traversal, and then (before we start
actually deleting anything) check to make sure that each targeted
partition-dependent object has been reached via a partition-type
dependency, implying that at least one of its partition owners was
deleted. The existing data structure handles this nicely, we just
need a couple more flag bits for the specific need. (BTW, I experimented
with whether we could handle internal dependencies the same way; but
it didn't work, because of the previously-poorly-documented fact that
an internal dependency is immediately turned into a reverse-direction
recursion. We can't really muck with the dependency traversal order,
or we find ourselves deleting tables before their indices, etc.)
* I found two different ways in which dependency.c was still producing
traversal-order-sensitive results. One we already understood is that the
first loop in findDependentObjects threw an error for the first internal
or partition dependency it came across; since an object can have both of
those, the results varied. This was fixed by postponing the actual error
report to after the loop and adding an explicit preference order for what
to report.
* The other such issue is a pre-existing bug, which maybe we ought to
back-patch, though I can't recall seeing any field reports that seem
to match it: after recursing to an internal/extension dependency,
we need to ensure that whatever objflags were passed down to our level
get applied to the targetObjects entry for the current object. Otherwise
the final state of the entry can vary depending on traversal order
(since orders in which the current call sees the object already in
targetObjects, or on the stack, would apply the objflags). This also
provides a chance to verify, not just assume, that processing of the
internal/extension dependency deleted the current object. As I mentioned
the other day, failing to ensure that the current object gets deleted
is very bad, and the pg_depend network is no longer so immutable that
we really ought to just assume that control came back to our object.
* The changes outside dependency.c just amount to applying the rules
stated above about how to use partition dependencies. I reverted the
places where v11 had decided that partition dependencies could be
substituted for auto dependencies, and made sure they are always
created in pairs.
There are a couple of things I didn't do here because those parts
of the patch were written while I still had some hope of applying
it to v11. Given that a catversion bump is needed anyway due to
the changes in what pg_depend entries are created for partitions,
we could change these:
* I renamed the dependency type to DEPENDENCY_PARTITION internally,
but left its pg_depend representation as 'I'. In a green field
we'd probably have made it 'P' instead.
* We could get rid of partition_dependency_matches(), which is not
really anything but a kluge, by splitting DEPENDENCY_PARTITION
into two dependency types, say DEPENDENCY_PARTITION_PRI ('P')
and DEPENDENCY_PARTITION_SEC ('S').
The main argument against changing these would be the risk that
client code already knows about 'I'. But neither pg_dump nor psql
do, so I judge that probably there's little if anything out there
that is special-casing that dependency type.
Also, I came across some coding in CloneFkReferencing() that looks fishy
as hell: that function imagines that it can delete an existing trigger
with nothing more than a summary CatalogTupleDelete(). I didn't do
anything about that here, but if it's not broken, I'd like to see an
explanation why not. I added a comment complaining about the lack of
pg_depend cleanup, and there's also the question of whether we don't
need to broadcast a relcache inval for the trigger's table.
regards, tom lane
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index af4d062..7731d46 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
*************** SCRAM-SHA-256$<replaceable><iteration
*** 2970,2976 ****
referenced object, and should be automatically dropped
(regardless of <literal>RESTRICT</literal> or <literal>CASCADE</literal>
mode) if the referenced object is dropped. Example: a named
! constraint on a table is made autodependent on the table, so
that it will go away if the table is dropped.
</para>
</listitem>
--- 2970,2976 ----
referenced object, and should be automatically dropped
(regardless of <literal>RESTRICT</literal> or <literal>CASCADE</literal>
mode) if the referenced object is dropped. Example: a named
! constraint on a table is made auto-dependent on the table, so
that it will go away if the table is dropped.
</para>
</listitem>
*************** SCRAM-SHA-256$<replaceable><iteration
*** 2982,3019 ****
<para>
The dependent object was created as part of creation of the
referenced object, and is really just a part of its internal
! implementation. A <command>DROP</command> of the dependent object
! will be disallowed outright (we'll tell the user to issue a
! <command>DROP</command> against the referenced object, instead). A
! <command>DROP</command> of the referenced object will be propagated
! through to drop the dependent object whether
! <command>CASCADE</command> is specified or not. Example: a trigger
! that's created to enforce a foreign-key constraint is made
! internally dependent on the constraint's
! <structname>pg_constraint</structname> entry.
</para>
</listitem>
</varlistentry>
<varlistentry>
! <term><symbol>DEPENDENCY_INTERNAL_AUTO</symbol> (<literal>I</literal>)</term>
<listitem>
<para>
The dependent object was created as part of creation of the
referenced object, and is really just a part of its internal
! implementation. A <command>DROP</command> of the dependent object
! will be disallowed outright (we'll tell the user to issue a
! <command>DROP</command> against the referenced object, instead).
! While a regular internal dependency will prevent
! the dependent object from being dropped while any such dependencies
! remain, <literal>DEPENDENCY_INTERNAL_AUTO</literal> will allow such
! a drop as long as the object can be found by following any of such
! dependencies.
! Example: an index on a partition is made internal-auto-dependent on
! both the partition itself as well as on the index on the parent
! partitioned table; so the partition index is dropped together with
! either the partition it indexes, or with the parent index it is
! attached to.
</para>
</listitem>
</varlistentry>
--- 2982,3032 ----
<para>
The dependent object was created as part of creation of the
referenced object, and is really just a part of its internal
! implementation. A direct <command>DROP</command> of the dependent
! object will be disallowed outright (we'll tell the user to issue
! a <command>DROP</command> against the referenced object, instead).
! A <command>DROP</command> of the referenced object will result in
! automatically dropping the dependent object
! whether <literal>CASCADE</literal> is specified or not. If the
! dependent object is reached due to a dependency on some other object,
! the drop is converted to a drop of the referenced object, so
! that <literal>NORMAL</literal> and <literal>AUTO</literal>
! dependencies of the dependent object behave much like they were
! dependencies of the referenced object.
! Example: a view's <literal>ON SELECT</literal> rule is made
! internally dependent on the view, preventing it from being dropped
! while the view remains. Dependencies of the rule (such as tables it
! refers to) act as if they were dependencies of the view.
</para>
</listitem>
</varlistentry>
<varlistentry>
! <term><symbol>DEPENDENCY_PARTITION</symbol> (<literal>I</literal>)</term>
<listitem>
<para>
The dependent object was created as part of creation of the
referenced object, and is really just a part of its internal
! implementation; however, unlike <literal>INTERNAL</literal>,
! there is more than one such referenced object. The dependent object
! must not be dropped unless at least one of these referenced objects
! is dropped; if any one is, the dependent object should be dropped
! whether or not <literal>CASCADE</literal> is specified. Also
! unlike <literal>INTERNAL</literal>, a drop of some other object
! that the dependent object depends on does not result in automatic
! deletion of any partition-referenced object. Hence, if the drop
! does not cascade to at least one of these objects via some other
! path, it will be refused. (In most cases, the dependent object
! shares all its non-partition dependencies with at least one
! partition-referenced object, so that this restriction does not
! result in blocking any cascaded delete.)
! Note that partition dependencies are made in addition to, not
! instead of, any dependencies the object would normally have. This
! simplifies <command>ATTACH/DETACH PARTITION</command> operations:
! the partition dependencies need only be added or removed.
! Example: a child partitioned index is made partition-dependent
! on both the partition table it is on and the parent partitioned
! index, so that it goes away if either of those is dropped.
</para>
</listitem>
</varlistentry>
*************** SCRAM-SHA-256$<replaceable><iteration
*** 3026,3034 ****
the referenced object (see
<link linkend="catalog-pg-extension"><structname>pg_extension</structname></link>).
The dependent object can be dropped only via
! <command>DROP EXTENSION</command> on the referenced object. Functionally
! this dependency type acts the same as an internal dependency, but
! it's kept separate for clarity and to simplify <application>pg_dump</application>.
</para>
</listitem>
</varlistentry>
--- 3039,3048 ----
the referenced object (see
<link linkend="catalog-pg-extension"><structname>pg_extension</structname></link>).
The dependent object can be dropped only via
! <command>DROP EXTENSION</command> on the referenced object.
! Functionally this dependency type acts the same as
! an <literal>INTERNAL</literal> dependency, but it's kept separate for
! clarity and to simplify <application>pg_dump</application>.
</para>
</listitem>
</varlistentry>
*************** SCRAM-SHA-256$<replaceable><iteration
*** 3038,3047 ****
<listitem>
<para>
The dependent object is not a member of the extension that is the
! referenced object (and so should not be ignored by pg_dump), but
! cannot function without it and should be dropped when the
! extension itself is. The dependent object may be dropped on its
! own as well.
</para>
</listitem>
</varlistentry>
--- 3052,3064 ----
<listitem>
<para>
The dependent object is not a member of the extension that is the
! referenced object (and so it should not be ignored
! by <application>pg_dump</application>), but it cannot function
! without the extension and should be auto-dropped if the extension is.
! The dependent object may be dropped on its own as well.
! Functionally this dependency type acts the same as
! an <literal>AUTO</literal> dependency, but it's kept separate for
! clarity and to simplify <application>pg_dump</application>.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 2c54895..702dbf3 100644
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
*************** typedef struct
*** 99,107 ****
#define DEPFLAG_NORMAL 0x0002 /* reached via normal dependency */
#define DEPFLAG_AUTO 0x0004 /* reached via auto dependency */
#define DEPFLAG_INTERNAL 0x0008 /* reached via internal dependency */
! #define DEPFLAG_EXTENSION 0x0010 /* reached via extension dependency */
! #define DEPFLAG_REVERSE 0x0020 /* reverse internal/extension link */
! #define DEPFLAG_SUBOBJECT 0x0040 /* subobject of another deletable object */
/* expansible list of ObjectAddresses */
--- 99,109 ----
#define DEPFLAG_NORMAL 0x0002 /* reached via normal dependency */
#define DEPFLAG_AUTO 0x0004 /* reached via auto dependency */
#define DEPFLAG_INTERNAL 0x0008 /* reached via internal dependency */
! #define DEPFLAG_PARTITION 0x0010 /* reached via partition dependency */
! #define DEPFLAG_EXTENSION 0x0020 /* reached via extension dependency */
! #define DEPFLAG_REVERSE 0x0040 /* reverse internal/extension link */
! #define DEPFLAG_IS_PART 0x0080 /* has a partition dependency */
! #define DEPFLAG_SUBOBJECT 0x0100 /* subobject of another deletable object */
/* expansible list of ObjectAddresses */
*************** static void reportDependentObjects(const
*** 194,199 ****
--- 196,203 ----
DropBehavior behavior,
int flags,
const ObjectAddress *origObject);
+ static bool partition_dependency_matches(const ObjectAddress *object1,
+ const ObjectAddress *object2);
static void deleteOneObject(const ObjectAddress *object,
Relation *depRel, int32 flags);
static void doDeletion(const ObjectAddress *object, int flags);
*************** findDependentObjects(const ObjectAddress
*** 478,483 ****
--- 482,489 ----
SysScanDesc scan;
HeapTuple tup;
ObjectAddress otherObject;
+ ObjectAddress owningObject;
+ ObjectAddress partitionObject;
ObjectAddressAndFlags *dependentObjects;
int numDependentObjects;
int maxDependentObjects;
*************** findDependentObjects(const ObjectAddress
*** 547,552 ****
--- 553,562 ----
scan = systable_beginscan(*depRel, DependDependerIndexId, true,
NULL, nkeys, key);
+ /* initialize variables that loop may fill */
+ memset(&owningObject, 0, sizeof(owningObject));
+ memset(&partitionObject, 0, sizeof(partitionObject));
+
while (HeapTupleIsValid(tup = systable_getnext(scan)))
{
Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(tup);
*************** findDependentObjects(const ObjectAddress
*** 591,614 ****
/* FALL THRU */
case DEPENDENCY_INTERNAL:
- case DEPENDENCY_INTERNAL_AUTO:
/*
* This object is part of the internal implementation of
* another object, or is part of the extension that is the
* other object. We have three cases:
*
! * 1. At the outermost recursion level, disallow the DROP. (We
! * just ereport here, rather than proceeding, since no other
! * dependencies are likely to be interesting.) However, if
! * the owning object is listed in pendingObjects, just release
! * the caller's lock and return; we'll eventually complete the
! * DROP when we reach that entry in the pending list.
*/
if (stack == NULL)
{
- char *otherObjDesc;
-
if (pendingObjects &&
object_address_present(&otherObject, pendingObjects))
{
--- 601,626 ----
/* FALL THRU */
case DEPENDENCY_INTERNAL:
/*
* This object is part of the internal implementation of
* another object, or is part of the extension that is the
* other object. We have three cases:
*
! * 1. At the outermost recursion level, we must disallow the
! * DROP. However, if the owning object is listed in
! * pendingObjects, just release the caller's lock and return;
! * we'll eventually complete the DROP when we reach that entry
! * in the pending list.
! *
! * Note: the above statement is true only if this pg_depend
! * entry still exists by then; in principle, therefore, we
! * could miss deleting an item the user told us to delete.
! * However, no inconsistency can result: since we're at outer
! * level, there is no object depending on this one.
*/
if (stack == NULL)
{
if (pendingObjects &&
object_address_present(&otherObject, pendingObjects))
{
*************** findDependentObjects(const ObjectAddress
*** 617,630 ****
ReleaseDeletionLock(object);
return;
}
! otherObjDesc = getObjectDescription(&otherObject);
! ereport(ERROR,
! (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
! errmsg("cannot drop %s because %s requires it",
! getObjectDescription(object),
! otherObjDesc),
! errhint("You can drop %s instead.",
! otherObjDesc)));
}
/*
--- 629,645 ----
ReleaseDeletionLock(object);
return;
}
!
! /*
! * We postpone actually issuing the error message until
! * after this loop, so that we can make the behavior
! * independent of the ordering of pg_depend entries, at
! * least so long as there's just one INTERNAL + EXTENSION
! * dependency. (If there's more, we'll complain about a
! * random one of them.)
! */
! owningObject = otherObject;
! break;
}
/*
*************** findDependentObjects(const ObjectAddress
*** 643,656 ****
* transform this deletion request into a delete of this
* owning object.
*
- * For INTERNAL_AUTO dependencies, we don't enforce this; in
- * other words, we don't follow the links back to the owning
- * object.
- */
- if (foundDep->deptype == DEPENDENCY_INTERNAL_AUTO)
- break;
-
- /*
* First, release caller's lock on this object and get
* deletion lock on the owning object. (We must release
* caller's lock to avoid deadlock against a concurrent
--- 658,663 ----
*************** findDependentObjects(const ObjectAddress
*** 673,678 ****
--- 680,692 ----
}
/*
+ * One way or the other, we're done with the scan; might as
+ * well close it down before recursing, to reduce peak
+ * resource consumption.
+ */
+ systable_endscan(scan);
+
+ /*
* Okay, recurse to the owning object instead of proceeding.
*
* We do not need to stack the current object; we want the
*************** findDependentObjects(const ObjectAddress
*** 690,699 ****
targetObjects,
pendingObjects,
depRel);
/* And we're done here. */
- systable_endscan(scan);
return;
case DEPENDENCY_PIN:
/*
--- 704,750 ----
targetObjects,
pendingObjects,
depRel);
+
+ /*
+ * We should have added the current target object while
+ * processing the owning object; if not, something's very
+ * wrong. While we're checking that, also make sure that the
+ * current object gets marked with whatever objflags we were
+ * given, just as though it'd been in targetObjects when we
+ * got here. (Otherwise, we might get a bogus failure from
+ * reportDependentObjects.)
+ */
+ if (!object_address_present_add_flags(object, objflags,
+ targetObjects))
+ elog(ERROR, "deletion of owning object %s failed to delete %s",
+ getObjectDescription(&otherObject),
+ getObjectDescription(object));
+
/* And we're done here. */
return;
+ case DEPENDENCY_PARTITION:
+
+ /*
+ * If this is the "most important" partition dependency,
+ * remember it for possible use in the error message. We
+ * consider the most important dependency to be one of the
+ * same classId (and relkind, if they're relations) as the
+ * target object. There should be exactly one such
+ * dependency, or we'll report a random one of them...
+ */
+ if (!(objflags & DEPFLAG_IS_PART) ||
+ partition_dependency_matches(object, &otherObject))
+ partitionObject = otherObject;
+
+ /*
+ * Remember that this object has a partition-type dependency.
+ * After the dependency scan, we'll complain if we didn't find
+ * a reason to delete one of its partition dependencies.
+ */
+ objflags |= DEPFLAG_IS_PART;
+ break;
+
case DEPENDENCY_PIN:
/*
*************** findDependentObjects(const ObjectAddress
*** 713,718 ****
--- 764,791 ----
systable_endscan(scan);
/*
+ * If we found an INTERNAL or EXTENSION dependency when we're at outer
+ * level, complain about it now. If we also found a PARTITION dependency,
+ * we prefer to report the PARTITION dependency. This is arbitrary but
+ * seems to be more useful in practice.
+ */
+ if (OidIsValid(owningObject.classId))
+ {
+ char *otherObjDesc;
+
+ if (OidIsValid(partitionObject.classId))
+ otherObjDesc = getObjectDescription(&partitionObject);
+ else
+ otherObjDesc = getObjectDescription(&owningObject);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("cannot drop %s because %s requires it",
+ getObjectDescription(object), otherObjDesc),
+ errhint("You can drop %s instead.", otherObjDesc)));
+ }
+
+ /*
* Next, identify all objects that directly depend on the current object.
* To ensure predictable deletion order, we collect them up in
* dependentObjects and sort the list before actually recursing. (The
*************** findDependentObjects(const ObjectAddress
*** 789,798 ****
case DEPENDENCY_AUTO_EXTENSION:
subflags = DEPFLAG_AUTO;
break;
- case DEPENDENCY_INTERNAL_AUTO:
case DEPENDENCY_INTERNAL:
subflags = DEPFLAG_INTERNAL;
break;
case DEPENDENCY_EXTENSION:
subflags = DEPFLAG_EXTENSION;
break;
--- 862,873 ----
case DEPENDENCY_AUTO_EXTENSION:
subflags = DEPFLAG_AUTO;
break;
case DEPENDENCY_INTERNAL:
subflags = DEPFLAG_INTERNAL;
break;
+ case DEPENDENCY_PARTITION:
+ subflags = DEPFLAG_PARTITION;
+ break;
case DEPENDENCY_EXTENSION:
subflags = DEPFLAG_EXTENSION;
break;
*************** findDependentObjects(const ObjectAddress
*** 868,877 ****
/*
* Finally, we can add the target object to targetObjects. Be careful to
* include any flags that were passed back down to us from inner recursion
! * levels.
*/
extra.flags = mystack.flags;
! if (stack)
extra.dependee = *stack->object;
else
memset(&extra.dependee, 0, sizeof(extra.dependee));
--- 943,957 ----
/*
* Finally, we can add the target object to targetObjects. Be careful to
* include any flags that were passed back down to us from inner recursion
! * levels. Record the "dependee" as being either the most important
! * partition owner if there is one, else the object we recursed from, if
! * any. (The logic in reportDependentObjects() is such that it can only
! * need one of those objects.)
*/
extra.flags = mystack.flags;
! if (extra.flags & DEPFLAG_IS_PART)
! extra.dependee = partitionObject;
! else if (stack)
extra.dependee = *stack->object;
else
memset(&extra.dependee, 0, sizeof(extra.dependee));
*************** reportDependentObjects(const ObjectAddre
*** 906,913 ****
int i;
/*
* If no error is to be thrown, and the msglevel is too low to be shown to
! * either client or server log, there's no need to do any of the work.
*
* Note: this code doesn't know all there is to be known about elog
* levels, but it works for NOTICE and DEBUG2, which are the only values
--- 986,1022 ----
int i;
/*
+ * If we need to delete any partition-dependent objects, make sure that
+ * we're deleting at least one of their partition dependencies, too. That
+ * can be detected by checking that we reached them by a PARTITION
+ * dependency at some point.
+ *
+ * We just report the first such object, as in most cases the only way to
+ * trigger this complaint is to explicitly try to delete one partition of
+ * a partitioned object.
+ */
+ for (i = 0; i < targetObjects->numrefs; i++)
+ {
+ const ObjectAddressExtra *extra = &targetObjects->extras[i];
+
+ if ((extra->flags & DEPFLAG_IS_PART) &&
+ !(extra->flags & DEPFLAG_PARTITION))
+ {
+ const ObjectAddress *object = &targetObjects->refs[i];
+ char *otherObjDesc = getObjectDescription(&extra->dependee);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("cannot drop %s because %s requires it",
+ getObjectDescription(object), otherObjDesc),
+ errhint("You can drop %s instead.", otherObjDesc)));
+ }
+ }
+
+ /*
* If no error is to be thrown, and the msglevel is too low to be shown to
! * either client or server log, there's no need to do any of the rest of
! * the work.
*
* Note: this code doesn't know all there is to be known about elog
* levels, but it works for NOTICE and DEBUG2, which are the only values
*************** reportDependentObjects(const ObjectAddre
*** 951,961 ****
/*
* If, at any stage of the recursive search, we reached the object via
! * an AUTO, INTERNAL, or EXTENSION dependency, then it's okay to
! * delete it even in RESTRICT mode.
*/
if (extra->flags & (DEPFLAG_AUTO |
DEPFLAG_INTERNAL |
DEPFLAG_EXTENSION))
{
/*
--- 1060,1071 ----
/*
* If, at any stage of the recursive search, we reached the object via
! * an AUTO, INTERNAL, PARTITION, or EXTENSION dependency, then it's
! * okay to delete it even in RESTRICT mode.
*/
if (extra->flags & (DEPFLAG_AUTO |
DEPFLAG_INTERNAL |
+ DEPFLAG_PARTITION |
DEPFLAG_EXTENSION))
{
/*
*************** reportDependentObjects(const ObjectAddre
*** 1063,1068 ****
--- 1173,1208 ----
}
/*
+ * Decide whether object1 and object2 are "the same type" of object.
+ * This is a bit of a hack.
+ */
+ static bool
+ partition_dependency_matches(const ObjectAddress *object1,
+ const ObjectAddress *object2)
+ {
+ if (object1->classId == object2->classId)
+ {
+ if (object1->classId == RelationRelationId)
+ {
+ char relkind1 = get_rel_relkind(object1->objectId);
+ char relkind2 = get_rel_relkind(object2->objectId);
+
+ /* Indexes and partitioned indexes are the same type of object */
+ if (relkind1 == RELKIND_PARTITIONED_INDEX)
+ relkind1 = RELKIND_INDEX;
+ if (relkind2 == RELKIND_PARTITIONED_INDEX)
+ relkind2 = RELKIND_INDEX;
+
+ if (relkind1 == relkind2)
+ return true;
+ }
+ else
+ return true;
+ }
+ return false;
+ }
+
+ /*
* deleteOneObject: delete a single object for performDeletion.
*
* *depRel is the already-open pg_depend relation.
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index faf6956..fb01999 100644
*** a/src/backend/catalog/index.c
--- b/src/backend/catalog/index.c
*************** index_create(Relation heapRelation,
*** 1041,1049 ****
else
{
bool have_simple_col = false;
- DependencyType deptype;
-
- deptype = OidIsValid(parentIndexRelid) ? DEPENDENCY_INTERNAL_AUTO : DEPENDENCY_AUTO;
/* Create auto dependencies on simply-referenced columns */
for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
--- 1041,1046 ----
*************** index_create(Relation heapRelation,
*** 1054,1060 ****
referenced.objectId = heapRelationId;
referenced.objectSubId = indexInfo->ii_IndexAttrNumbers[i];
! recordDependencyOn(&myself, &referenced, deptype);
have_simple_col = true;
}
--- 1051,1057 ----
referenced.objectId = heapRelationId;
referenced.objectSubId = indexInfo->ii_IndexAttrNumbers[i];
! recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
have_simple_col = true;
}
*************** index_create(Relation heapRelation,
*** 1072,1089 ****
referenced.objectId = heapRelationId;
referenced.objectSubId = 0;
! recordDependencyOn(&myself, &referenced, deptype);
}
}
! /* Store dependency on parent index, if any */
if (OidIsValid(parentIndexRelid))
{
referenced.classId = RelationRelationId;
referenced.objectId = parentIndexRelid;
referenced.objectSubId = 0;
! recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL_AUTO);
}
/* Store dependency on collations */
--- 1069,1097 ----
referenced.objectId = heapRelationId;
referenced.objectSubId = 0;
! recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
}
}
! /*
! * If this is an index partition, create partition dependencies on
! * both the parent index and the table. (Note: these must be *in
! * addition to*, not instead of, all other dependencies. Otherwise
! * we'll be short some dependencies after DETACH PARTITION.)
! */
if (OidIsValid(parentIndexRelid))
{
referenced.classId = RelationRelationId;
referenced.objectId = parentIndexRelid;
referenced.objectSubId = 0;
! recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION);
!
! referenced.classId = RelationRelationId;
! referenced.objectId = heapRelationId;
! referenced.objectSubId = 0;
!
! recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION);
}
/* Store dependency on collations */
*************** index_constraint_create(Relation heapRel
*** 1342,1356 ****
recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL);
/*
! * Also, if this is a constraint on a partition, mark it as depending on
! * the constraint in the parent.
*/
if (OidIsValid(parentConstraintId))
{
! ObjectAddress parentConstr;
!
! ObjectAddressSet(parentConstr, ConstraintRelationId, parentConstraintId);
! recordDependencyOn(&referenced, &parentConstr, DEPENDENCY_INTERNAL_AUTO);
}
/*
--- 1350,1366 ----
recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL);
/*
! * Also, if this is a constraint on a partition, give it partition-type
! * dependencies on the parent constraint as well as the table.
*/
if (OidIsValid(parentConstraintId))
{
! ObjectAddressSet(myself, ConstraintRelationId, conOid);
! ObjectAddressSet(referenced, ConstraintRelationId, parentConstraintId);
! recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION);
! ObjectAddressSet(referenced, RelationRelationId,
! RelationGetRelid(heapRelation));
! recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION);
}
/*
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 698b493..4023c3d 100644
*** a/src/backend/catalog/pg_constraint.c
--- b/src/backend/catalog/pg_constraint.c
*************** AlterConstraintNamespaces(Oid ownerId, O
*** 760,772 ****
/*
* ConstraintSetParentConstraint
! * Set a partition's constraint as child of its parent table's
*
* This updates the constraint's pg_constraint row to show it as inherited, and
! * add a dependency to the parent so that it cannot be removed on its own.
*/
void
! ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
{
Relation constrRel;
Form_pg_constraint constrForm;
--- 760,776 ----
/*
* ConstraintSetParentConstraint
! * Set a partition's constraint as child of its parent constraint,
! * or remove the linkage if parentConstrId is InvalidOid.
*
* This updates the constraint's pg_constraint row to show it as inherited, and
! * adds PARTITION dependencies to prevent the constraint from being deleted
! * on its own. Alternatively, reverse that.
*/
void
! ConstraintSetParentConstraint(Oid childConstrId,
! Oid parentConstrId,
! Oid childTableId)
{
Relation constrRel;
Form_pg_constraint constrForm;
*************** ConstraintSetParentConstraint(Oid childC
*** 795,804 ****
CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
- ObjectAddressSet(referenced, ConstraintRelationId, parentConstrId);
ObjectAddressSet(depender, ConstraintRelationId, childConstrId);
! recordDependencyOn(&depender, &referenced, DEPENDENCY_INTERNAL_AUTO);
}
else
{
--- 799,811 ----
CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
ObjectAddressSet(depender, ConstraintRelationId, childConstrId);
! ObjectAddressSet(referenced, ConstraintRelationId, parentConstrId);
! recordDependencyOn(&depender, &referenced, DEPENDENCY_PARTITION);
!
! ObjectAddressSet(referenced, RelationRelationId, childTableId);
! recordDependencyOn(&depender, &referenced, DEPENDENCY_PARTITION);
}
else
{
*************** ConstraintSetParentConstraint(Oid childC
*** 809,818 ****
/* Make sure there's no further inheritance. */
Assert(constrForm->coninhcount == 0);
deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId,
ConstraintRelationId,
! DEPENDENCY_INTERNAL_AUTO);
! CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
}
ReleaseSysCache(tuple);
--- 816,829 ----
/* Make sure there's no further inheritance. */
Assert(constrForm->coninhcount == 0);
+ CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
+
deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId,
ConstraintRelationId,
! DEPENDENCY_PARTITION);
! deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId,
! RelationRelationId,
! DEPENDENCY_PARTITION);
}
ReleaseSysCache(tuple);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index bd85099..207e1ff 100644
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
*************** DefineIndex(Oid relationId,
*** 971,977 ****
IndexSetParentIndex(cldidx, indexRelationId);
if (createdConstraintId != InvalidOid)
ConstraintSetParentConstraint(cldConstrOid,
! createdConstraintId);
if (!cldidx->rd_index->indisvalid)
invalidate_parent = true;
--- 971,978 ----
IndexSetParentIndex(cldidx, indexRelationId);
if (createdConstraintId != InvalidOid)
ConstraintSetParentConstraint(cldConstrOid,
! createdConstraintId,
! childRelid);
if (!cldidx->rd_index->indisvalid)
invalidate_parent = true;
*************** IndexSetParentIndex(Relation partitionId
*** 2622,2656 ****
if (fix_dependencies)
{
- ObjectAddress partIdx;
-
/*
! * Insert/delete pg_depend rows. If setting a parent, add an
! * INTERNAL_AUTO dependency to the parent index; if making standalone,
! * remove all existing rows and put back the regular dependency on the
! * table.
*/
- ObjectAddressSet(partIdx, RelationRelationId, partRelid);
-
if (OidIsValid(parentOid))
{
ObjectAddress parentIdx;
ObjectAddressSet(parentIdx, RelationRelationId, parentOid);
! recordDependencyOn(&partIdx, &parentIdx, DEPENDENCY_INTERNAL_AUTO);
}
else
{
- ObjectAddress partitionTbl;
-
- ObjectAddressSet(partitionTbl, RelationRelationId,
- partitionIdx->rd_index->indrelid);
-
deleteDependencyRecordsForClass(RelationRelationId, partRelid,
RelationRelationId,
! DEPENDENCY_INTERNAL_AUTO);
!
! recordDependencyOn(&partIdx, &partitionTbl, DEPENDENCY_AUTO);
}
/* make our updates visible */
--- 2623,2651 ----
if (fix_dependencies)
{
/*
! * Insert/delete pg_depend rows. If setting a parent, add PARTITION
! * dependencies on the parent index and the table; if removing a
! * parent, delete PARTITION dependencies.
*/
if (OidIsValid(parentOid))
{
+ ObjectAddress partIdx;
ObjectAddress parentIdx;
+ ObjectAddress partitionTbl;
+ ObjectAddressSet(partIdx, RelationRelationId, partRelid);
ObjectAddressSet(parentIdx, RelationRelationId, parentOid);
! ObjectAddressSet(partitionTbl, RelationRelationId,
! partitionIdx->rd_index->indrelid);
! recordDependencyOn(&partIdx, &parentIdx, DEPENDENCY_PARTITION);
! recordDependencyOn(&partIdx, &partitionTbl, DEPENDENCY_PARTITION);
}
else
{
deleteDependencyRecordsForClass(RelationRelationId, partRelid,
RelationRelationId,
! DEPENDENCY_PARTITION);
}
/* make our updates visible */
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 35a9ade..f4d95e0 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** CloneFkReferencing(Relation pg_constrain
*** 7825,7831 ****
bool attach_it;
Oid constrOid;
ObjectAddress parentAddr,
! childAddr;
ListCell *cell;
int i;
--- 7825,7832 ----
bool attach_it;
Oid constrOid;
ObjectAddress parentAddr,
! childAddr,
! childTableAddr;
ListCell *cell;
int i;
*************** CloneFkReferencing(Relation pg_constrain
*** 7945,7957 ****
trgform->oid,
ConstraintRelationId,
DEPENDENCY_INTERNAL);
CatalogTupleDelete(trigrel, &trigtup->t_self);
}
systable_endscan(scan);
table_close(trigrel, RowExclusiveLock);
! ConstraintSetParentConstraint(fk->conoid, parentConstrOid);
CommandCounterIncrement();
attach_it = true;
break;
--- 7946,7963 ----
trgform->oid,
ConstraintRelationId,
DEPENDENCY_INTERNAL);
+ /*
+ * XXX surely this is an entirely inadequate way to delete
+ * a trigger. What of other dependencies, for example?
+ */
CatalogTupleDelete(trigrel, &trigtup->t_self);
}
systable_endscan(scan);
table_close(trigrel, RowExclusiveLock);
! ConstraintSetParentConstraint(fk->conoid, parentConstrOid,
! RelationGetRelid(partRel));
CommandCounterIncrement();
attach_it = true;
break;
*************** CloneFkReferencing(Relation pg_constrain
*** 7998,8005 ****
1, false, true);
subclone = lappend_oid(subclone, constrOid);
ObjectAddressSet(childAddr, ConstraintRelationId, constrOid);
! recordDependencyOn(&childAddr, &parentAddr, DEPENDENCY_INTERNAL_AUTO);
fkconstraint = makeNode(Constraint);
/* for now this is all we need */
--- 8004,8015 ----
1, false, true);
subclone = lappend_oid(subclone, constrOid);
+ /* Set up partition dependencies for the new constraint */
ObjectAddressSet(childAddr, ConstraintRelationId, constrOid);
! recordDependencyOn(&childAddr, &parentAddr, DEPENDENCY_PARTITION);
! ObjectAddressSet(childTableAddr, RelationRelationId,
! RelationGetRelid(partRel));
! recordDependencyOn(&childAddr, &childTableAddr, DEPENDENCY_PARTITION);
fkconstraint = makeNode(Constraint);
/* for now this is all we need */
*************** AttachPartitionEnsureIndexes(Relation re
*** 14878,14884 ****
/* bingo. */
IndexSetParentIndex(attachrelIdxRels[i], idx);
if (OidIsValid(constraintOid))
! ConstraintSetParentConstraint(cldConstrOid, constraintOid);
update_relispartition(NULL, cldIdxId, true);
found = true;
break;
--- 14888,14895 ----
/* bingo. */
IndexSetParentIndex(attachrelIdxRels[i], idx);
if (OidIsValid(constraintOid))
! ConstraintSetParentConstraint(cldConstrOid, constraintOid,
! RelationGetRelid(attachrel));
update_relispartition(NULL, cldIdxId, true);
found = true;
break;
*************** ATExecDetachPartition(Relation rel, Rang
*** 15136,15142 ****
constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
idxid);
if (OidIsValid(constrOid))
! ConstraintSetParentConstraint(constrOid, InvalidOid);
index_close(idx, NoLock);
}
--- 15147,15153 ----
constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
idxid);
if (OidIsValid(constrOid))
! ConstraintSetParentConstraint(constrOid, InvalidOid, InvalidOid);
index_close(idx, NoLock);
}
*************** ATExecDetachPartition(Relation rel, Rang
*** 15168,15174 ****
}
/* unset conparentid and adjust conislocal, coninhcount, etc. */
! ConstraintSetParentConstraint(fk->conoid, InvalidOid);
/*
* Make the action triggers on the referenced relation. When this was
--- 15179,15185 ----
}
/* unset conparentid and adjust conislocal, coninhcount, etc. */
! ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
/*
* Make the action triggers on the referenced relation. When this was
*************** ATExecAttachPartitionIdx(List **wqueue,
*** 15404,15410 ****
/* All good -- do it */
IndexSetParentIndex(partIdx, RelationGetRelid(parentIdx));
if (OidIsValid(constraintOid))
! ConstraintSetParentConstraint(cldConstrId, constraintOid);
update_relispartition(NULL, partIdxId, true);
pfree(attmap);
--- 15415,15422 ----
/* All good -- do it */
IndexSetParentIndex(partIdx, RelationGetRelid(parentIdx));
if (OidIsValid(constraintOid))
! ConstraintSetParentConstraint(cldConstrId, constraintOid,
! RelationGetRelid(partTbl));
update_relispartition(NULL, partIdxId, true);
pfree(attmap);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 0b245a6..5e6437a 100644
*** a/src/backend/commands/trigger.c
--- b/src/backend/commands/trigger.c
*************** CreateTrigger(CreateTrigStmt *stmt, cons
*** 1012,1028 ****
* User CREATE TRIGGER, so place dependencies. We make trigger be
* auto-dropped if its relation is dropped or if the FK relation is
* dropped. (Auto drop is compatible with our pre-7.3 behavior.)
- *
- * Exception: if this trigger comes from a parent partitioned table,
- * then it's not separately drop-able, but goes away if the partition
- * does.
*/
referenced.classId = RelationRelationId;
referenced.objectId = RelationGetRelid(rel);
referenced.objectSubId = 0;
! recordDependencyOn(&myself, &referenced, OidIsValid(parentTriggerOid) ?
! DEPENDENCY_INTERNAL_AUTO :
! DEPENDENCY_AUTO);
if (OidIsValid(constrrelid))
{
--- 1012,1022 ----
* User CREATE TRIGGER, so place dependencies. We make trigger be
* auto-dropped if its relation is dropped or if the FK relation is
* dropped. (Auto drop is compatible with our pre-7.3 behavior.)
*/
referenced.classId = RelationRelationId;
referenced.objectId = RelationGetRelid(rel);
referenced.objectSubId = 0;
! recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
if (OidIsValid(constrrelid))
{
*************** CreateTrigger(CreateTrigStmt *stmt, cons
*** 1046,1056 ****
recordDependencyOn(&referenced, &myself, DEPENDENCY_INTERNAL);
}
! /* Depends on the parent trigger, if there is one. */
if (OidIsValid(parentTriggerOid))
{
ObjectAddressSet(referenced, TriggerRelationId, parentTriggerOid);
! recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL_AUTO);
}
}
--- 1040,1054 ----
recordDependencyOn(&referenced, &myself, DEPENDENCY_INTERNAL);
}
! /*
! * If it's a partition trigger, create the partition dependencies.
! */
if (OidIsValid(parentTriggerOid))
{
ObjectAddressSet(referenced, TriggerRelationId, parentTriggerOid);
! recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION);
! ObjectAddressSet(referenced, RelationRelationId, RelationGetRelid(rel));
! recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION);
}
}
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 5dea270..ad7cc78 100644
*** a/src/include/catalog/dependency.h
--- b/src/include/catalog/dependency.h
***************
*** 24,87 ****
*
* In all cases, a dependency relationship indicates that the referenced
* object may not be dropped without also dropping the dependent object.
! * However, there are several subflavors:
! *
! * DEPENDENCY_NORMAL ('n'): normal relationship between separately-created
! * objects. The dependent object may be dropped without affecting the
! * referenced object. The referenced object may only be dropped by
! * specifying CASCADE, in which case the dependent object is dropped too.
! * Example: a table column has a normal dependency on its datatype.
! *
! * DEPENDENCY_AUTO ('a'): the dependent object can be dropped separately
! * from the referenced object, and should be automatically dropped
! * (regardless of RESTRICT or CASCADE mode) if the referenced object
! * is dropped.
! * Example: a named constraint on a table is made auto-dependent on
! * the table, so that it will go away if the table is dropped.
! *
! * DEPENDENCY_INTERNAL ('i'): the dependent object was created as part
! * of creation of the referenced object, and is really just a part of
! * its internal implementation. A DROP of the dependent object will be
! * disallowed outright (we'll tell the user to issue a DROP against the
! * referenced object, instead). A DROP of the referenced object will be
! * propagated through to drop the dependent object whether CASCADE is
! * specified or not.
! * Example: a trigger that's created to enforce a foreign-key constraint
! * is made internally dependent on the constraint's pg_constraint entry.
! *
! * DEPENDENCY_INTERNAL_AUTO ('I'): the dependent object was created as
! * part of creation of the referenced object, and is really just a part
! * of its internal implementation. A DROP of the dependent object will
! * be disallowed outright (we'll tell the user to issue a DROP against the
! * referenced object, instead). While a regular internal dependency will
! * prevent the dependent object from being dropped while any such
! * dependencies remain, DEPENDENCY_INTERNAL_AUTO will allow such a drop as
! * long as the object can be found by following any of such dependencies.
! * Example: an index on a partition is made internal-auto-dependent on
! * both the partition itself as well as on the index on the parent
! * partitioned table; so the partition index is dropped together with
! * either the partition it indexes, or with the parent index it is attached
! * to.
!
! * DEPENDENCY_EXTENSION ('e'): the dependent object is a member of the
! * extension that is the referenced object. The dependent object can be
! * dropped only via DROP EXTENSION on the referenced object. Functionally
! * this dependency type acts the same as an internal dependency, but it's
! * kept separate for clarity and to simplify pg_dump.
! *
! * DEPENDENCY_AUTO_EXTENSION ('x'): the dependent object is not a member
! * of the extension that is the referenced object (and so should not be
! * ignored by pg_dump), but cannot function without the extension and
! * should be dropped when the extension itself is. The dependent object
! * may be dropped on its own as well.
! *
! * DEPENDENCY_PIN ('p'): there is no dependent object; this type of entry
! * is a signal that the system itself depends on the referenced object,
! * and so that object must never be deleted. Entries of this type are
! * created only during initdb. The fields for the dependent object
! * contain zeroes.
! *
! * Other dependency flavors may be needed in future.
*/
typedef enum DependencyType
--- 24,31 ----
*
* In all cases, a dependency relationship indicates that the referenced
* object may not be dropped without also dropping the dependent object.
! * However, there are several subflavors; see the description of pg_depend
! * in catalogs.sgml for details.
*/
typedef enum DependencyType
*************** typedef enum DependencyType
*** 89,95 ****
DEPENDENCY_NORMAL = 'n',
DEPENDENCY_AUTO = 'a',
DEPENDENCY_INTERNAL = 'i',
! DEPENDENCY_INTERNAL_AUTO = 'I',
DEPENDENCY_EXTENSION = 'e',
DEPENDENCY_AUTO_EXTENSION = 'x',
DEPENDENCY_PIN = 'p'
--- 33,39 ----
DEPENDENCY_NORMAL = 'n',
DEPENDENCY_AUTO = 'a',
DEPENDENCY_INTERNAL = 'i',
! DEPENDENCY_PARTITION = 'I',
DEPENDENCY_EXTENSION = 'e',
DEPENDENCY_AUTO_EXTENSION = 'x',
DEPENDENCY_PIN = 'p'
diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 55a2694..c87bded 100644
*** a/src/include/catalog/pg_constraint.h
--- b/src/include/catalog/pg_constraint.h
*************** extern char *ChooseConstraintName(const
*** 239,245 ****
extern void AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
Oid newNspId, bool isType, ObjectAddresses *objsMoved);
extern void ConstraintSetParentConstraint(Oid childConstrId,
! Oid parentConstrId);
extern Oid get_relation_constraint_oid(Oid relid, const char *conname, bool missing_ok);
extern Bitmapset *get_relation_constraint_attnos(Oid relid, const char *conname,
bool missing_ok, Oid *constraintOid);
--- 239,246 ----
extern void AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
Oid newNspId, bool isType, ObjectAddresses *objsMoved);
extern void ConstraintSetParentConstraint(Oid childConstrId,
! Oid parentConstrId,
! Oid childTableId);
extern Oid get_relation_constraint_oid(Oid relid, const char *conname, bool missing_ok);
extern Bitmapset *get_relation_constraint_attnos(Oid relid, const char *conname,
bool missing_ok, Oid *constraintOid);
pgsql-hackers by date: