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...@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