Re: Issue in ExecCleanupTupleRouting() - Mailing list pgsql-hackers

From David Rowley
Subject Re: Issue in ExecCleanupTupleRouting()
Date
Msg-id CAKJS1f85t_pXv3y4C5La9C+_8kQjAEfSsu6eZOo_a7jQ3HxrJA@mail.gmail.com
Whole thread Raw
In response to Issue in ExecCleanupTupleRouting()  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
Responses Re: Issue in ExecCleanupTupleRouting()
List pgsql-hackers
On Fri, 12 Apr 2019 at 01:06, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
> +       /*
> +        * Check if this result rel is one belonging to the node's subplans,
> +        * if so, let ExecEndPlan() clean it up.
> +        */
> +       if (htab)
> +       {
> +           Oid         partoid;
> +           bool        found;
> +
> +           partoid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
> +
> +           (void) hash_search(htab, &partoid, HASH_FIND, &found);
> +           if (found)
> +               continue;
> +       }
>
>         /* Allow any FDWs to shut down if they've been exercised */
> -       if (resultRelInfo->ri_PartitionReadyForRouting &&
> -           resultRelInfo->ri_FdwRoutine != NULL &&
> +       if (resultRelInfo->ri_FdwRoutine != NULL &&
>             resultRelInfo->ri_FdwRoutine->EndForeignInsert != NULL)
>
> resultRelInfo->ri_FdwRoutine->EndForeignInsert(mtstate->ps.state,
>                                                            resultRelInfo);
>
> This skips subplan resultrels before calling EndForeignInsert() if they
> are foreign tables, which I think causes an issue: the FDWs would fail
> to release resources for their foreign insert operations, because
> ExecEndPlan() and ExecEndModifyTable() don't do anything to allow them
> to do that.  So I think we should skip subplan resultrels after
> EndForeignInsert().  Attached is a small patch for that.

Oops. I had for some reason been under the impression that it was
nodeModifyTable.c, or whatever the calling code happened to be that
handles these ones, but this is not the case as we call
ExecInitRoutingInfo() from ExecFindPartition() which makes the call to
BeginForeignInsert. If that part is handled by the tuple routing code,
then the subsequent cleanup should be too, in which case your patch
looks fine.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: finding changed blocks using WAL scanning
Next
From: Michael Paquier
Date:
Subject: Re: pg_rewind vs superuser