Re: pg_restore causing deadlocks on partitioned tables - Mailing list pgsql-hackers

On Thu, Sep 17, 2020 at 3:40 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Langote <amitlangote09@gmail.com> writes:
> > Updated patch attached.
>
> Pushed with a little bit of fooling about.

Thank you.

>  After looking at the
> git history, I saw that the Assert we were wondering about used to
> be just "Assert(constr)", and there were not run-time checks on
> whether constr is null.  That was changed when f0e44751d added
> partition constraint checking into ExecConstraints' responsibilities.
> At some later point that code was removed from ExecConstraints,
> but we failed to undo the other changes in ExecConstraints, leaving
> it looking pretty silly.  So I reverted this to the way it was,
> with just an Assert and no regular checks.
>
> I also did a bit more work on the comments.  (Speaking of which,
> is there a better place to put the commentary you removed from
> InitResultRelInfo?  It was surely wildly out of place there,
> but I'm wondering if maybe we have a README that should cover it.)

Actually, the two points of interest in that now removed comment,
which was this:

-    * Partition constraint, which also includes the partition constraint of
-    * all the ancestors that are partitions.  Note that it will be checked
-    * even in the case of tuple-routing where this table is the target leaf
-    * partition, if there any BR triggers defined on the table.  Although
-    * tuple-routing implicitly preserves the partition constraint of the
-    * target partition for a given row, the BR triggers may change the row
-    * such that the constraint is no longer satisfied, which we must fail for
-    * by checking it explicitly.
-    *
-    * If this is a partitioned table, the partition constraint (if any) of a
-    * given row will be checked just before performing tuple-routing.

are also mentioned, although in less words, where they are relevant:

In ExecInsert():

        /*
         * Also check the tuple against the partition constraint, if there is
         * one; except that if we got here via tuple-routing, we don't need to
         * if there's no BR trigger defined on the partition.
         */
        if (resultRelationDesc->rd_rel->relispartition &&
            (resultRelInfo->ri_PartitionRoot == NULL ||
             (resultRelInfo->ri_TrigDesc &&
              resultRelInfo->ri_TrigDesc->trig_insert_before_row)))
            ExecPartitionCheck(resultRelInfo, slot, estate, true);

In ExecFindPartition():

    /*
     * First check the root table's partition constraint, if any.  No point in
     * routing the tuple if it doesn't belong in the root table itself.
     */
    if (rootResultRelInfo->ri_RelationDesc->rd_rel->relispartition)
        ExecPartitionCheck(rootResultRelInfo, slot, estate, true);

Maybe that's enough?

-- 
Amit Langote
EnterpriseDB: http://www.enterprisedb.com