Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Multi-insert into a partitioned table with before insert row trigger causes server crash on latest HEAD
Date: 2018-10-20 05:45:23
Message-ID: CAKJS1f-b3TQ7OxNLsM0WevVigZB6nTyTpsoUhtirDtLKeiAiDQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(Returns from leave and beyond the reaches of the internet)

On 18 October 2018 at 07:45, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> On 16/10/2018 06:33, Ashutosh Sharma wrote:
>> I think, the root cause of this problem is that CopyFrom() is using
>> the stale value of *has_before_insert_row_trig* to determine if the
>> current partition is okay for multi-insert or not i.e.
>> has_before_insert_row_trig used to determine multi-insert condition
>> for the current partition actually belongs to old partition. I think,
>> *has_before_insert_row_trig* needs to updated before CopyFrom()
>> evaluates if the current partition is good to go for multi insert or
>> not. Attached is the patch based on this. I've also added the relevant
>> test-case for it. Peter, David, Could you please have a look into the
>> attached patch and share your thoughts. Thank you.
>
> I have committed your fix and test, moving some code around a bit. Thanks.

Thanks for pushing that fix.

Originally my patch in [1] only could set leafpart_use_multi_insert to
true within the `if (insertMethod == CIM_MULTI_CONDITIONAL)` test, so
wouldn't have suffered from this problem.

I'm not sure that doubling up the `insertMethod ==
CIM_MULTI_CONDITIONAL` test is the cleanest fix. Personally, I liked
the way it was in the v6 edition of the patch, but I'm used to getting
outvoted.

[1] https://www.postgresql.org/message-id/CAKJS1f9f8yuj04X_rffNu2JPbvhy+YP_aVH6iwCTJ1OL=YwCOg@mail.gmail.com

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2018-10-20 05:53:38 Re: pgsql: Add TAP tests for pg_verify_checksums
Previous Message Andres Freund 2018-10-20 05:16:05 Re: pgsql: Add TAP tests for pg_verify_checksums