Re: a misbehavior of partition row movement (?) - Mailing list pgsql-hackers

On Fri, Sep 10, 2021 at 11:03 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Fri, Sep 3, 2021 at 12:23 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > Hi Andrew,
> >
> > On Fri, Sep 3, 2021 at 6:19 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> > > On 7/13/21 8:09 AM, Amit Langote wrote:
> > > > Unfortunately, I don’t think I’ll have time in this CF to solve some
> > > > very fundamental issues I found in the patch during the last cycle.
> > > > I’m fine with either marking this as RwF for now or move to the next CF.
> > >
> > > Amit, do you have time now to work on this?
> >
> > I will take some time next week to take a fresh look at this and post an update.
>
> So I started looking at this today.  I didn't make much an inroad into
> the stumbling block with 0002 patch that I had mentioned back in [1],
> though I decided to at least post a rebased version of the patches
> that apply.
>
> I think 0001 is independently committable on its own merits,
> irrespective of the yet unresolved problems of 0002, a patch to fix
> $subject, which I'll continue to work on.
>
> 0003 shows a test that crashes the server due to said problem.

I think I found a solution to the problem with 0002.

The problem was that the tuplestore
(afterTriggers.query_stack[query_level].tuplestore) that I decided to
use to store the AFTER trigger tuples of a partitioned table that is
the target of an cross-partition update lives only for the duration of
a given query.  So that approach wouldn't work if the foreign key
pointing into that partitioned table is marked INITIALLY DEFERRED.  To
fix, I added a List field to AfterTriggersData that stores the
tuplestores to store the tuples of partitioned tables that undergo
cross-partition updates in a transaction and are pointed to by
INITIALLY DEFERRED foreign key constraints.  I couldn't understand one
comment about why using a tuplestore for such cases *might not* work,
which as follows:

 * Note that we need triggers on foreign tables to be fired in exactly the
 * order they were queued, so that the tuples come out of the tuplestore in
 * the right order.  To ensure that, we forbid deferrable (constraint)
 * triggers on foreign tables.  This also ensures that such triggers do not
 * get deferred into outer trigger query levels, meaning that it's okay to
 * destroy the tuplestore at the end of the query level.

I tried to break the approach using various test cases (some can be
seen added by the patch to foreign_key.sql), but couldn't see the
issue alluded to in the above comment.  So I've marked the comment
with an XXX note as follows:

- * Note that we need triggers on foreign tables to be fired in exactly the
- * order they were queued, so that the tuples come out of the tuplestore in
- * the right order.  To ensure that, we forbid deferrable (constraint)
- * triggers on foreign tables.  This also ensures that such triggers do not
- * get deferred into outer trigger query levels, meaning that it's okay to
- * destroy the tuplestore at the end of the query level.
+ * Note that we need triggers on foreign and partitioned tables to be fired in
+ * exactly the order they were queued, so that the tuples come out of the
+ * tuplestore in the right order.  To ensure that, we forbid deferrable
+ * (constraint) triggers on foreign tables.  This also ensures that such
+ * triggers do not get deferred into outer trigger query levels, meaning that
+ * it's okay to destroy the tuplestore at the end of the query level.
+ * XXX - update this paragraph if the new approach, whereby tuplestores in
+ * afterTriggers.deferred_tuplestores outlive any given query, can be proven
+ * to not really break any assumptions mentioned here.

If anyone reading this can think of the issue the original comment
seems to be talking about, please let me know.

I've attached updated patches.  I've addressed Zhihong Yu's comment too.

Thank you.

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