]> granicus.if.org Git - postgresql/commitdiff
Redesign the partition dependency mechanism.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 11 Feb 2019 19:41:13 +0000 (14:41 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 11 Feb 2019 19:41:17 +0000 (14:41 -0500)
The original setup for dependencies of partitioned objects had
serious problems:

1. It did not verify that a drop cascading to a partition-child object
also cascaded to at least one of the object's partition parents.  Now,
normally a child object would share all its dependencies with one or
another parent (e.g. a child index's opclass dependencies would be shared
with the parent index), so that this oversight is usually harmless.
But if some dependency failed to fit this pattern, the child could be
dropped while all its parents remain, creating a logically broken
situation.  (It's easy to construct artificial cases that break it,
such as attaching an unrelated extension dependency to the child object
and then dropping the extension.  I'm not sure if any less-artificial
cases exist.)

2. Management of partition dependencies during ATTACH/DETACH PARTITION
was complicated and buggy; for example, after detaching a partition
table it was possible to create cases where a formerly-child index
should be dropped and was not, because the correct set of dependencies
had not been reconstructed.

Less seriously, because multiple partition relationships were
represented identically in pg_depend, there was an order-of-traversal
dependency on which partition parent was cited in error messages.
We also had some pre-existing order-of-traversal hazards for error
messages related to internal and extension dependencies.  This is
cosmetic to users but causes testing problems.

To fix #1, add a check at the end of the partition tree traversal
to ensure that at least one partition parent got deleted.  To fix #2,
establish a new policy that partition dependencies are in addition to,
not instead of, a child object's usual dependencies; in this way
ATTACH/DETACH PARTITION need not cope with adding or removing the
usual dependencies.

To fix the cosmetic problem, distinguish between primary and secondary
partition dependency entries in pg_depend, by giving them different
deptypes.  (They behave identically except for having different
priorities for being cited in error messages.)  This means that the
former 'I' dependency type is replaced with new 'P' and 'S' types.

This also fixes a longstanding bug that after handling an internal
dependency by recursing to the owning object, findDependentObjects
did not verify that the current target was now scheduled for deletion,
and did not apply the current recursion level's objflags to it.
Perhaps that should be back-patched; but in the back branches it
would only matter if some concurrent transaction had removed the
internal-linkage pg_depend entry before the recursive call found it,
or the recursive call somehow failed to find it, both of which seem
unlikely.

Catversion bump because the contents of pg_depend change for
partitioning relationships.

Patch HEAD only.  It's annoying that we're not fixing #2 in v11,
but there seems no practical way to do so given that the problem
is exactly a poor choice of what entries to put in pg_depend.
We can't really fix that while staying compatible with what's
in pg_depend in existing v11 installations.

Discussion: https://postgr.es/m/CAH2-Wzkypv1R+teZrr71U23J578NnTBt2X8+Y=Odr4pOdW1rXg@mail.gmail.com

12 files changed:
doc/src/sgml/catalogs.sgml
src/backend/catalog/dependency.c
src/backend/catalog/index.c
src/backend/catalog/pg_constraint.c
src/backend/commands/indexcmds.c
src/backend/commands/tablecmds.c
src/backend/commands/trigger.c
src/include/catalog/catversion.h
src/include/catalog/dependency.h
src/include/catalog/pg_constraint.h
src/test/regress/expected/indexing.out
src/test/regress/sql/indexing.sql

index 6dd0700da752619209c1cb33036918e33a8753f5..0fd792ff1a2f92adc451be4329df3a56a9c1b90d 100644 (file)
@@ -2970,7 +2970,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
        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
+       constraint on a table is made auto-dependent on the table, so
        that it will go away if the table is dropped.
       </para>
      </listitem>
@@ -2982,38 +2982,61 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
       <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.
+       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_INTERNAL_AUTO</symbol> (<literal>I</literal>)</term>
+     <term><symbol>DEPENDENCY_PARTITION_PRI</symbol> (<literal>P</literal>)</term>
+     <term><symbol>DEPENDENCY_PARTITION_SEC</symbol> (<literal>S</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
+       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.)
+       Primary and secondary partition dependencies behave identically
+       except that the primary dependency is preferred for use in error
+       messages; hence, a partition-dependent object should have one
+       primary partition dependency and one or more secondary partition
        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.
+       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, but
+       not otherwise.  The dependency on the parent index is primary,
+       so that if the user tries to drop the child partitioned index,
+       the error message will suggest dropping the parent index instead
+       (not the table).
       </para>
      </listitem>
     </varlistentry>
@@ -3026,9 +3049,10 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
        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>.
+       <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>
@@ -3038,10 +3062,13 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
      <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.
+       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>
@@ -3063,6 +3090,19 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
    Other dependency flavors might be needed in future.
   </para>
 
+  <para>
+   Note that it's quite possible for two objects to be linked by more than
+   one <structname>pg_depend</structname> entry.  For example, a child
+   partitioned index would have both a partition-type dependency on its
+   associated partition table, and an auto dependency on each column of
+   that table that it indexes.  This sort of situation expresses the union
+   of multiple dependency semantics.  A dependent object can be dropped
+   without <literal>CASCADE</literal> if any of its dependencies satisfies
+   its condition for automatic dropping.  Conversely, all the
+   dependencies' restrictions about which objects must be dropped together
+   must be satisfied.
+  </para>
+
  </sect1>
 
 
index 2c548958313d4476807581c17401fc71ba6101f9..2048d71535bf2903b7ea93d89aa4449e598897eb 100644 (file)
@@ -99,9 +99,11 @@ typedef struct
 #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 */
+#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 */
@@ -478,6 +480,8 @@ findDependentObjects(const ObjectAddress *object,
        SysScanDesc scan;
        HeapTuple       tup;
        ObjectAddress otherObject;
+       ObjectAddress owningObject;
+       ObjectAddress partitionObject;
        ObjectAddressAndFlags *dependentObjects;
        int                     numDependentObjects;
        int                     maxDependentObjects;
@@ -547,6 +551,10 @@ findDependentObjects(const ObjectAddress *object,
        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);
@@ -591,24 +599,26 @@ findDependentObjects(const ObjectAddress *object,
                                /* 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.
+                                * 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)
                                {
-                                       char       *otherObjDesc;
-
                                        if (pendingObjects &&
                                                object_address_present(&otherObject, pendingObjects))
                                        {
@@ -617,14 +627,21 @@ findDependentObjects(const ObjectAddress *object,
                                                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)));
+
+                                       /*
+                                        * 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 if there's not more than one INTERNAL and one
+                                        * EXTENSION dependency.  (If there's more, we'll complain
+                                        * about a random one of them.)  Prefer to complain about
+                                        * EXTENSION, since that's generally a more important
+                                        * dependency.
+                                        */
+                                       if (!OidIsValid(owningObject.classId) ||
+                                               foundDep->deptype == DEPENDENCY_EXTENSION)
+                                               owningObject = otherObject;
+                                       break;
                                }
 
                                /*
@@ -643,14 +660,6 @@ findDependentObjects(const ObjectAddress *object,
                                 * 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
@@ -672,6 +681,13 @@ findDependentObjects(const ObjectAddress *object,
                                        return;
                                }
 
+                               /*
+                                * 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.
                                 *
@@ -690,10 +706,66 @@ findDependentObjects(const ObjectAddress *object,
                                                                         targetObjects,
                                                                         pendingObjects,
                                                                         depRel);
+
+                               /*
+                                * The current target object should have been added to
+                                * targetObjects while processing the owning object; but it
+                                * probably got only the flag bits associated with the
+                                * dependency we're looking at.  We need to add the objflags
+                                * that were passed to this recursion level, too, else we may
+                                * get a bogus failure in reportDependentObjects (if, for
+                                * example, we were called due to a partition dependency).
+                                *
+                                * If somehow the current object didn't get scheduled for
+                                * deletion, bleat.  (That would imply that somebody deleted
+                                * this dependency record before the recursion got to it.)
+                                * Another idea would be to reacquire lock on the current
+                                * object and resume trying to delete it, but it seems not
+                                * worth dealing with the race conditions inherent in that.
+                                */
+                               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. */
-                               systable_endscan(scan);
                                return;
 
+                       case DEPENDENCY_PARTITION_PRI:
+
+                               /*
+                                * 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;
+
+                               /*
+                                * Also remember the primary partition owner, for error
+                                * messages.  If there are multiple primary owners (which
+                                * there should not be), we'll report a random one of them.
+                                */
+                               partitionObject = otherObject;
+                               break;
+
+                       case DEPENDENCY_PARTITION_SEC:
+
+                               /*
+                                * Only use secondary partition owners in error messages if we
+                                * find no primary owner (which probably shouldn't happen).
+                                */
+                               if (!(objflags & DEPFLAG_IS_PART))
+                                       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:
 
                                /*
@@ -712,6 +784,28 @@ findDependentObjects(const ObjectAddress *object,
 
        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
@@ -789,10 +883,13 @@ findDependentObjects(const ObjectAddress *object,
                        case DEPENDENCY_AUTO_EXTENSION:
                                subflags = DEPFLAG_AUTO;
                                break;
-                       case DEPENDENCY_INTERNAL_AUTO:
                        case DEPENDENCY_INTERNAL:
                                subflags = DEPFLAG_INTERNAL;
                                break;
+                       case DEPENDENCY_PARTITION_PRI:
+                       case DEPENDENCY_PARTITION_SEC:
+                               subflags = DEPFLAG_PARTITION;
+                               break;
                        case DEPENDENCY_EXTENSION:
                                subflags = DEPFLAG_EXTENSION;
                                break;
@@ -868,10 +965,15 @@ findDependentObjects(const ObjectAddress *object,
        /*
         * 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.
+        * 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 (stack)
+       if (extra.flags & DEPFLAG_IS_PART)
+               extra.dependee = partitionObject;
+       else if (stack)
                extra.dependee = *stack->object;
        else
                memset(&extra.dependee, 0, sizeof(extra.dependee));
@@ -905,9 +1007,38 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
        int                     numNotReportedClient = 0;
        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 work.
+        * 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
@@ -951,11 +1082,12 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
 
                /*
                 * 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.
+                * 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))
                {
                        /*
index faf69568133677bc98672fdef1f8fc8e1d891f54..d16c3d0ea50f28044b18e5740ac5e1d1cc3e280d 100644 (file)
@@ -1041,9 +1041,6 @@ index_create(Relation heapRelation,
                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++)
@@ -1054,7 +1051,7 @@ index_create(Relation heapRelation,
                                        referenced.objectId = heapRelationId;
                                        referenced.objectSubId = indexInfo->ii_IndexAttrNumbers[i];
 
-                                       recordDependencyOn(&myself, &referenced, deptype);
+                                       recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
 
                                        have_simple_col = true;
                                }
@@ -1072,18 +1069,29 @@ index_create(Relation heapRelation,
                                referenced.objectId = heapRelationId;
                                referenced.objectSubId = 0;
 
-                               recordDependencyOn(&myself, &referenced, deptype);
+                               recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
                        }
                }
 
-               /* Store dependency on parent index, if any */
+               /*
+                * 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_INTERNAL_AUTO);
+                       recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION_PRI);
+
+                       referenced.classId = RelationRelationId;
+                       referenced.objectId = heapRelationId;
+                       referenced.objectSubId = 0;
+
+                       recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION_SEC);
                }
 
                /* Store dependency on collations */
@@ -1342,15 +1350,17 @@ index_constraint_create(Relation heapRelation,
        recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL);
 
        /*
-        * Also, if this is a constraint on a partition, mark it as depending on
-        * the constraint in the parent.
+        * 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))
        {
-               ObjectAddress parentConstr;
-
-               ObjectAddressSet(parentConstr, ConstraintRelationId, parentConstraintId);
-               recordDependencyOn(&referenced, &parentConstr, DEPENDENCY_INTERNAL_AUTO);
+               ObjectAddressSet(myself, ConstraintRelationId, conOid);
+               ObjectAddressSet(referenced, ConstraintRelationId, parentConstraintId);
+               recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION_PRI);
+               ObjectAddressSet(referenced, RelationRelationId,
+                                                RelationGetRelid(heapRelation));
+               recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION_SEC);
        }
 
        /*
index 698b493fc409e410b7ad534d32a5f30c0804d7b4..ad836e0d98e501102ecf78747e606b86eb8c4d2e 100644 (file)
@@ -760,13 +760,17 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
 
 /*
  * ConstraintSetParentConstraint
- *             Set a partition's constraint as child of its parent table's
+ *             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
- * add a dependency to the parent so that it cannot be removed on its own.
+ * adds PARTITION dependencies to prevent the constraint from being deleted
+ * on its own.  Alternatively, reverse that.
  */
 void
-ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
+ConstraintSetParentConstraint(Oid childConstrId,
+                                                         Oid parentConstrId,
+                                                         Oid childTableId)
 {
        Relation        constrRel;
        Form_pg_constraint constrForm;
@@ -795,10 +799,13 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
 
                CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
 
-               ObjectAddressSet(referenced, ConstraintRelationId, parentConstrId);
                ObjectAddressSet(depender, ConstraintRelationId, childConstrId);
 
-               recordDependencyOn(&depender, &referenced, DEPENDENCY_INTERNAL_AUTO);
+               ObjectAddressSet(referenced, ConstraintRelationId, parentConstrId);
+               recordDependencyOn(&depender, &referenced, DEPENDENCY_PARTITION_PRI);
+
+               ObjectAddressSet(referenced, RelationRelationId, childTableId);
+               recordDependencyOn(&depender, &referenced, DEPENDENCY_PARTITION_SEC);
        }
        else
        {
@@ -809,10 +816,14 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
                /* Make sure there's no further inheritance. */
                Assert(constrForm->coninhcount == 0);
 
+               CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
+
                deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId,
                                                                                ConstraintRelationId,
-                                                                               DEPENDENCY_INTERNAL_AUTO);
-               CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
+                                                                               DEPENDENCY_PARTITION_PRI);
+               deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId,
+                                                                               RelationRelationId,
+                                                                               DEPENDENCY_PARTITION_SEC);
        }
 
        ReleaseSysCache(tuple);
index bd85099c28624a82265411cafaf0c52e71766ce2..7352b9e341d3138626d358b262d2f6a5ad39706f 100644 (file)
@@ -971,7 +971,8 @@ DefineIndex(Oid relationId,
                                                IndexSetParentIndex(cldidx, indexRelationId);
                                                if (createdConstraintId != InvalidOid)
                                                        ConstraintSetParentConstraint(cldConstrOid,
-                                                                                                                 createdConstraintId);
+                                                                                                                 createdConstraintId,
+                                                                                                                 childRelid);
 
                                                if (!cldidx->rd_index->indisvalid)
                                                        invalidate_parent = true;
@@ -2622,35 +2623,34 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
 
        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.
+                * 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.
                 */
-               ObjectAddressSet(partIdx, RelationRelationId, partRelid);
-
                if (OidIsValid(parentOid))
                {
+                       ObjectAddress partIdx;
                        ObjectAddress parentIdx;
+                       ObjectAddress partitionTbl;
 
+                       ObjectAddressSet(partIdx, RelationRelationId, partRelid);
                        ObjectAddressSet(parentIdx, RelationRelationId, parentOid);
-                       recordDependencyOn(&partIdx, &parentIdx, DEPENDENCY_INTERNAL_AUTO);
+                       ObjectAddressSet(partitionTbl, RelationRelationId,
+                                                        partitionIdx->rd_index->indrelid);
+                       recordDependencyOn(&partIdx, &parentIdx,
+                                                          DEPENDENCY_PARTITION_PRI);
+                       recordDependencyOn(&partIdx, &partitionTbl,
+                                                          DEPENDENCY_PARTITION_SEC);
                }
                else
                {
-                       ObjectAddress partitionTbl;
-
-                       ObjectAddressSet(partitionTbl, RelationRelationId,
-                                                        partitionIdx->rd_index->indrelid);
-
                        deleteDependencyRecordsForClass(RelationRelationId, partRelid,
                                                                                        RelationRelationId,
-                                                                                       DEPENDENCY_INTERNAL_AUTO);
-
-                       recordDependencyOn(&partIdx, &partitionTbl, DEPENDENCY_AUTO);
+                                                                                       DEPENDENCY_PARTITION_PRI);
+                       deleteDependencyRecordsForClass(RelationRelationId, partRelid,
+                                                                                       RelationRelationId,
+                                                                                       DEPENDENCY_PARTITION_SEC);
                }
 
                /* make our updates visible */
index b66d194b80ddca834c170ab9ef1ce873eee43233..715c6a221cfc94b1eb2a1f972d6968b0d90e2590 100644 (file)
@@ -7825,7 +7825,8 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
                bool            attach_it;
                Oid                     constrOid;
                ObjectAddress parentAddr,
-                                       childAddr;
+                                       childAddr,
+                                       childTableAddr;
                ListCell   *cell;
                int                     i;
 
@@ -7966,7 +7967,8 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
                        systable_endscan(scan);
                        table_close(trigrel, RowExclusiveLock);
 
-                       ConstraintSetParentConstraint(fk->conoid, parentConstrOid);
+                       ConstraintSetParentConstraint(fk->conoid, parentConstrOid,
+                                                                                 RelationGetRelid(partRel));
                        CommandCounterIncrement();
                        attach_it = true;
                        break;
@@ -8013,8 +8015,14 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
                                                                  1, false, true);
                subclone = lappend_oid(subclone, constrOid);
 
+               /* Set up partition dependencies for the new constraint */
                ObjectAddressSet(childAddr, ConstraintRelationId, constrOid);
-               recordDependencyOn(&childAddr, &parentAddr, DEPENDENCY_INTERNAL_AUTO);
+               recordDependencyOn(&childAddr, &parentAddr,
+                                                  DEPENDENCY_PARTITION_PRI);
+               ObjectAddressSet(childTableAddr, RelationRelationId,
+                                                RelationGetRelid(partRel));
+               recordDependencyOn(&childAddr, &childTableAddr,
+                                                  DEPENDENCY_PARTITION_SEC);
 
                fkconstraint = makeNode(Constraint);
                /* for now this is all we need */
@@ -14893,7 +14901,8 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
                                /* bingo. */
                                IndexSetParentIndex(attachrelIdxRels[i], idx);
                                if (OidIsValid(constraintOid))
-                                       ConstraintSetParentConstraint(cldConstrOid, constraintOid);
+                                       ConstraintSetParentConstraint(cldConstrOid, constraintOid,
+                                                                                                 RelationGetRelid(attachrel));
                                update_relispartition(NULL, cldIdxId, true);
                                found = true;
                                break;
@@ -15151,7 +15160,7 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
                constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
                                                                                                        idxid);
                if (OidIsValid(constrOid))
-                       ConstraintSetParentConstraint(constrOid, InvalidOid);
+                       ConstraintSetParentConstraint(constrOid, InvalidOid, InvalidOid);
 
                index_close(idx, NoLock);
        }
@@ -15183,7 +15192,7 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
                }
 
                /* unset conparentid and adjust conislocal, coninhcount, etc. */
-               ConstraintSetParentConstraint(fk->conoid, InvalidOid);
+               ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
 
                /*
                 * Make the action triggers on the referenced relation.  When this was
@@ -15419,7 +15428,8 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
                /* All good -- do it */
                IndexSetParentIndex(partIdx, RelationGetRelid(parentIdx));
                if (OidIsValid(constraintOid))
-                       ConstraintSetParentConstraint(cldConstrId, constraintOid);
+                       ConstraintSetParentConstraint(cldConstrId, constraintOid,
+                                                                                 RelationGetRelid(partTbl));
                update_relispartition(NULL, partIdxId, true);
 
                pfree(attmap);
index 0b245a613e0d413110b0a9e072b6066cdafb7cb3..409bee24f89f20864523c068c22082d2a6efb1b1 100644 (file)
@@ -1012,17 +1012,11 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
                 * 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);
+               recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
 
                if (OidIsValid(constrrelid))
                {
@@ -1046,11 +1040,15 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
                        recordDependencyOn(&referenced, &myself, DEPENDENCY_INTERNAL);
                }
 
-               /* Depends on the parent trigger, if there is one. */
+               /*
+                * If it's a partition trigger, create the partition dependencies.
+                */
                if (OidIsValid(parentTriggerOid))
                {
                        ObjectAddressSet(referenced, TriggerRelationId, parentTriggerOid);
-                       recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL_AUTO);
+                       recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION_PRI);
+                       ObjectAddressSet(referenced, RelationRelationId, RelationGetRelid(rel));
+                       recordDependencyOn(&myself, &referenced, DEPENDENCY_PARTITION_SEC);
                }
        }
 
index 968a6800ddc51e9084b8f5d8051ea64d5868af7b..7a65fde762bb099d7d036d788a25e06390dcacf3 100644 (file)
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     201902092
+#define CATALOG_VERSION_NO     201902111
 
 #endif
index 5dea27016eb12ee96aecb9a4cf7bbb7a0fb4eb45..b235a23f5dc602b52bf9c5405757dedf54c9d99e 100644 (file)
  *
  * 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.
+ * However, there are several subflavors; see the description of pg_depend
+ * in catalogs.sgml for details.
  */
 
 typedef enum DependencyType
@@ -89,7 +33,8 @@ typedef enum DependencyType
        DEPENDENCY_NORMAL = 'n',
        DEPENDENCY_AUTO = 'a',
        DEPENDENCY_INTERNAL = 'i',
-       DEPENDENCY_INTERNAL_AUTO = 'I',
+       DEPENDENCY_PARTITION_PRI = 'P',
+       DEPENDENCY_PARTITION_SEC = 'S',
        DEPENDENCY_EXTENSION = 'e',
        DEPENDENCY_AUTO_EXTENSION = 'x',
        DEPENDENCY_PIN = 'p'
index 55a2694b101edd1566282e271a4ea4e3957e0315..c87bdedbbbe91c51ea849bd415a9c9f8a47552f4 100644 (file)
@@ -239,7 +239,8 @@ extern char *ChooseConstraintName(const char *name1, const char *name2,
 extern void AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
                                                  Oid newNspId, bool isType, ObjectAddresses *objsMoved);
 extern void ConstraintSetParentConstraint(Oid childConstrId,
-                                                         Oid parentConstrId);
+                                                         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);
index b344351ee7fd1cbf6d96c1c30a456f1ba11c2158..1f74cc4d4486cbf78d4c382232f9f54680fe332c 100644 (file)
@@ -528,6 +528,30 @@ select relname, relkind from pg_class where relname like 'idxpart%' order by rel
 ---------+---------
 (0 rows)
 
+create table idxpart (a int, b int, c int) partition by range(a);
+create index on idxpart(c);
+create table idxpart1 partition of idxpart for values from (0) to (250);
+create table idxpart2 partition of idxpart for values from (250) to (500);
+alter table idxpart detach partition idxpart2;
+\d idxpart2
+              Table "public.idxpart2"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ a      | integer |           |          | 
+ b      | integer |           |          | 
+ c      | integer |           |          | 
+Indexes:
+    "idxpart2_c_idx" btree (c)
+
+alter table idxpart2 drop column c;
+\d idxpart2
+              Table "public.idxpart2"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ a      | integer |           |          | 
+ b      | integer |           |          | 
+
+drop table idxpart, idxpart2;
 -- Verify that expression indexes inherit correctly
 create table idxpart (a int, b int) partition by range (a);
 create table idxpart1 (like idxpart);
index 3d43a57da22ee91dfe6ad4b6c5aeff28398949ca..42b4b03b817d65bd7e2bf5e2849c9abb093c7b1b 100644 (file)
@@ -249,6 +249,16 @@ select relname, relkind from pg_class where relname like 'idxpart%' order by rel
 drop table idxpart, idxpart1, idxpart2, idxpart3;
 select relname, relkind from pg_class where relname like 'idxpart%' order by relname;
 
+create table idxpart (a int, b int, c int) partition by range(a);
+create index on idxpart(c);
+create table idxpart1 partition of idxpart for values from (0) to (250);
+create table idxpart2 partition of idxpart for values from (250) to (500);
+alter table idxpart detach partition idxpart2;
+\d idxpart2
+alter table idxpart2 drop column c;
+\d idxpart2
+drop table idxpart, idxpart2;
+
 -- Verify that expression indexes inherit correctly
 create table idxpart (a int, b int) partition by range (a);
 create table idxpart1 (like idxpart);