Re: inserts into partitioned table may cause crash

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, thomas(dot)munro(at)enterprisedb(dot)com
Subject: Re: inserts into partitioned table may cause crash
Date: 2018-03-19 11:35:44
Message-ID: 5AAFA090.7030100@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2018/03/19 17:48), Amit Langote wrote:
> On 2018/03/16 20:37, Etsuro Fujita wrote:
>> (2018/03/14 17:25), Etsuro Fujita wrote:
>>> (2018/03/14 14:54), Amit Langote wrote:
>>>> Btw, I noticed that the patches place ExecPrepareTupleRouting (both the
>>>> declaration and the definition) at different relative locations within
>>>> nodeModifyTable.c in the HEAD branch vs. in the 10 branch. It might be a
>>>> good idea to bring them to the same relative location to avoid hassle
>>>> when
>>>> back-patching relevant patches in the future. I did that in the attached
>>>> updated version (v4) of the patch for HEAD, which does not make any other
>>>> changes. Although, the patch for PG-10 is unchanged, I still changed its
>>>> file name to contain v4.
>>>
>>> That's a good idea! Thanks for the updated patches!
>>
>> Sorry, I didn't look at those patches carefully, but I noticed that while
>> the patch for PG10 has put the definition of that function after the
>> definitions of functions such as fireBSTriggers, fireASTriggers and
>> ExecSetupTransitionCaptureState, but the patch for HEAD puts it *before*
>> the definitions of these functions (note: these functions don't have their
>> declarations at the file header), which seems a bit inconsistent. ISTM
>> that the v3 patches for PG10 and HEAD have already put that function in
>> the right place in terms of that relativity.
>
> That's correct, except v3 had added the definition of
> ExecPrepareTupleRouting after those of ExecSetupChildParentMapForSubplan,
> ExecSetupChildParentMapForTcs, etc., that are new in 11dev branch.
>
> I've fixed the patch for HEAD to move the ExecPrepareTupleRouting
> definition to appear after those of fireBSTriggers, fireASTriggers, and
> ExecSetupTransitionCaptureState.
>
> Attached are v5 patches for HEAD and 10 branches.

Thanks for the updated patches! I think the patches are in good shape,
but I did a bit of editorial things; added a bit more comments for
ExecPrepareTupleRouting and adjusted regression test stuff to match the
existing ones. Attached are the updated patches for HEAD and PG10.
Those changes are just editorial, so let's ask for the committer review.

Best regards,
Etsuro Fujita

Attachment Content-Type Size
ExecInsert-reset-estate-result-rel-10backpatch-v6.patch text/x-diff 14.3 KB
ExecInsert-reset-estate-result-rel-HEAD-v6.patch text/x-diff 16.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2018-03-19 11:50:48 Re: [HACKERS] GUC for cleanup indexes threshold.
Previous Message Amit Langote 2018-03-19 11:25:12 Re: [HACKERS] Add support for tuple routing to foreign partitions