Re: inserts into partitioned table may cause crash

Hi Amit,

(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 
from the
      * partition rowtype to parent rowtype.
      */
     if (mtstate->mt_transition_capture != NULL)
     {
         if (resultRelInfo->ri_TrigDesc &&
             (resultRelInfo->ri_TrigDesc->trig_insert_before_row ||
              resultRelInfo->ri_TrigDesc->trig_insert_instead_row))
         {
             /*
              * If there are any BEFORE or INSTEAD triggers on the 
partition,
              * we'll have to be ready to convert their result back to
              * tuplestore format.
              */
             mtstate->mt_transition_capture->tcs_original_insert_tuple = 
NULL;
             mtstate->mt_transition_capture->tcs_map =
                 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?

Best regards,
Etsuro Fujita