Re: Needless additional partition check in INSERT?

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.

Thanks,
Amit