Re: Needless additional partition check in INSERT?

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>
Subject: Re: Needless additional partition check in INSERT?
Date: 2018-06-07 06:02:31
Message-ID: CAKJS1f_=LvuZfLgP_8Y9yJMPKwB2RexVEpKNg4c2i4TKTzNWWw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 7 June 2018 at 17:45, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2018/06/07 13:10, David Rowley wrote:
>> On 7 June 2018 at 16:05, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>> Or we could just not have a separate function and put the logic that
>>> determines whether or not to check the partition constraint right before
>>> the following piece of code in ExecConstraints
>>>
>>> if (check_partition_constraint && resultRelInfo->ri_PartitionCheck &&
>>> !ExecPartitionCheck(resultRelInfo, slot, estate))
>>> ExecPartitionCheckEmitError(resultRelInfo, slot, estate);
>>
>> I don't think so. The function Alvaro is talking about is specific to
>> inserting a tuple into a table. The place you're proposing to put the
>> call to it is not.
>
> Well, we need to determine whether or not to check the partition
> constraint exactly before inserting a tuple into a table and that's also
> when ExecConstraints is called, so this objection is not clear to me.
>
> I'm saying that instead of encapsulating that logic in a new function and
> calling it from a number of places, refactor things such that we call
> ExecConstraints unconditionally and decide there which constraints to
> check and which ones to not. Having that logic separated from
> ExecConstraints is what caused us to have this discussion in the first place.
>
> I'm attaching a patch here to show what I'm saying.

I might not have fully explained what I meant. I'm guilty of thinking
it was pretty clear when I wrote my objection.

I meant:

ExecConstraints is not only called during INSERT and COPY TO, it's
also used during UPDATE [1].

What you're proposing seems to be adding a check for
trig_insert_before_row into a function that will be called during
UPDATE.

Can you explain why you think that's a good thing to do? I'm don't
really see why UPDATE should care about the presence, (or lack of) a
BEFORE ROW INSERT trigger.

[1] https://github.com/michaelpq/postgres/blob/master/src/backend/executor/nodeModifyTable.c#L1174

--
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 Amit Kapila 2018-06-07 06:14:57 Re: Concurrency bug in UPDATE of partition-key
Previous Message Amit Langote 2018-06-07 05:45:12 Re: Needless additional partition check in INSERT?