Re: non-bulk inserts and tuple routing

Hi,

On 2018-02-22 11:10:57 -0500, Robert Haas wrote:
> On Tue, Feb 20, 2018 at 8:06 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> >> Attached is an updated version for that.
> >
> > Thanks for updating the patch.
> 
> Committed with a few changes.  The big one was that I got rid of the
> local variable is_update in ExecSetupPartitionTupleRouting.  That
> saved a level of indentation on a substantial chunk of code, and it
> turns out that test was redundant anyway.

I noticed that this patch broke my JIT patch when I force JITing to be
used everywhere (obviously pointless for perf reasons, but good for
testing).  Turns out to be a single line.

ExecInitPartitionInfo has the following block:
        foreach(ll, wcoList)
        {
            WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll));
            ExprState  *wcoExpr = ExecInitQual(castNode(List, wco->qual),
                                               mtstate->mt_plans[0]);

            wcoExprs = lappend(wcoExprs, wcoExpr);
        }

note how it is passing mtstate->mt_plans[0] as the parent node for the
expression.  I don't quite know why mtstate->mt_plans[0] was chosen
here, it doesn't seem right. The WCO will not be executed in that
context. Note that the ExecBuildProjectionInfo() call a few lines below
also uses a different context.

For JITing that fails because the compiled deform will assume the tuples
look like mt_plans[0]'s scantuples. But we're not dealing with those,
we're dealing with tuples returned by the relevant subplan.

Also note that the code before this commit used
				ExprState  *wcoExpr = ExecInitQual(castNode(List, wco->qual),
												   &mtstate->ps);
i.e. the ModifyTableState node, as I'd expect.


Greetings,

Andres Freund