Re: Needless additional partition check in INSERT?

On 7 June 2018 at 17:45, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2018/06/07 13:10, David Rowley wrote:
>> On 7 June 2018 at 16:05, Amit Langote <Langote_Amit_f8@lab.ntt.co.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