|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>|
|Subject:||Re: inserts into partitioned table may cause crash|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
(2018/03/06 15:28), Amit Langote wrote:
> On 2018/03/05 22:00, Etsuro Fujita wrote:
>> An alternative fix for this would be to handle the set/reset of
>> estate->es_result_relation_info in a higher level ie, ExecModifyTable,
>> like the attached:
> Your patch seems like a good cleanup overall, fixes this bug, and seems to
> make future maintenance easier. So, +1. I agree that doing a surgery
> like this on COPY is better left for later.
Thanks for the review! I'll leave that for another patch, then.
> I noticed (as you may have too) that it cannot be applied to the 10 branch
> as is. I made the necessary changes so that it can be. See attached
> patch with filename suffixed "-10backpatch". Also attached is your patch
> unchanged to have both of them together.
Thanks for that!
One thing I notice while working on this is this in ExecInsert/CopyFrom,
which I moved to ExecPrepareTupleRouting as-is for the former:
* If we're capturing transition tuples, we might need to convert
* partition rowtype to parent rowtype.
if (mtstate->mt_transition_capture != NULL)
if (resultRelInfo->ri_TrigDesc &&
* If there are any BEFORE or INSTEAD triggers on the
* we'll have to be ready to convert their result back to
* tuplestore format.
TupConvMapForLeaf(proute, rootRelInfo, leaf_part_index);
Do we need to consider INSTEAD triggers here? The partition is either a
plain table or a foreign table, so I don't think it can have those
triggers. Am I missing something?
|Next Message||Ildus Kurbangaliev||2018-03-06 12:48:37||Re: committing inside cursor loop|
|Previous Message||Simon Riggs||2018-03-06 12:06:21||Re: Faster inserts with mostly-monotonically increasing values|