Re: inserts into partitioned table may cause crash - Mailing list pgsql-hackers

(2018/03/01 21:40), Etsuro Fujita wrote:
> (2018/02/28 17:36), Amit Langote wrote:
>> Attached a patch to fix that, which would need to be back-patched to 10.
>
> Good catch! Will review.

I started reviewing this.  I think the analysis you mentioned upthread 
would be correct, but I'm not sure the patch is the right way to go 
because I think that exception handling added by the patch throughout 
ExecInsert such as:

@@ -408,7 +408,10 @@ ExecInsert(ModifyTableState *mtstate,
                 slot = ExecBRInsertTriggers(estate, resultRelInfo, slot);

                 if (slot == NULL)               /* "do nothing" */
-                       return NULL;
+               {
+                       result = NULL;
+                       goto restore_estate_result_rel;
+               }

would reduce the code readability.

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: (1) before calling ExecInsert, we do the preparation 
work for tuple routing of one tuple (eg, determine the partition for the 
tuple and convert the format of the tuple in a separate function, which 
also sets estate->es_result_relation_info to the partition for 
ExecInsert to work on it; then (2) we call ExecInsert, which just 
inserts into the partition; then (3) we reset 
estate->es_result_relation_info back to the root partitioned table.  One 
good thing about the alternative is to return ExecInsert somehow to 
PG9.6, which would make maintenance easier.  COPY has the same issue, so 
the attached also fixes that.  (Maybe we could do some refactoring to 
use the same code in both cases, but I'm not sure we should do that as a 
fix.)  What do you think about the alternative?

Best regards,
Etsuro Fujita