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>
Subject: Re: inserts into partitioned table may cause crash
Date: 2018-03-05 13:00:58
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: 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

Attachment Content-Type Size
ExecInsert-reset-estate-result-rel-efujita.patch text/x-diff 12.0 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2018-03-05 13:03:37 Re: Transform for pl/perl
Previous Message David Fetter 2018-03-05 12:59:33 Re: Better Upgrades