Re: pg_restore causing deadlocks on partitioned tables

On Thu, Sep 17, 2020 at 3:40 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Langote <amitlangote09(at)gmail(dot)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