From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: non-bulk inserts and tuple routing |
Date: | 2018-03-05 02:21:46 |
Message-ID: | 58b10e27-70d2-5b51-2056-a7efc8a9ee2e@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2018/03/03 13:38, Andres Freund wrote:
> 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(at)lab(dot)ntt(dot)co(dot)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.
I guess that it was an oversight in my patch. Please find attached a
patch to fix that.
Thanks,
Amit
Attachment | Content-Type | Size |
---|---|---|
partition-WCO-qual-parent-fix.patch | text/plain | 535 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2018-03-05 02:56:00 | Re: [HACKERS] GUC for cleanup indexes threshold. |
Previous Message | Amit Langote | 2018-03-05 01:59:37 | Re: constraint exclusion and nulls in IN (..) clause |