Re: Needless additional partition check in INSERT?

On 11 May 2018 at 14:50, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
> On 10 May 2018 at 15:26, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
>> Yeah, the comments do need work. In order to make it a bit easier to
>> document I changed the way that check_partition_constr is set. This is
>> now done with an if/else if/else clause for both COPY and INSERT.
>>
>> Hopefully, that's easier to understand and prevents further mistakes.
>>
>> Patch attached.
>
> The patch looks good in terms of handling the redundant constraint check.
>
> Since this patch also does some minor code restructuring with the if
> conditions, below are some comments on them :
>
> With the patch, the if condition uses ri_PartitionCheck, and further
> ExecConstraints() also uses ri_PartitionCheck to decide whether to
> call ExecPartitionCheck(). So instead, if we just don't bother about
> ri_PartitionCheck outside ExecConstraints() and let ExecConstraints()
> handle it, then the if conditions would get simpler :
>
> if (resultRelInfo->ri_PartitionRoot == NULL ||
> (resultRelInfo->ri_TrigDesc &&
> resultRelInfo->ri_TrigDesc->trig_insert_before_row))
> check_partition_constr = true;
> else
> check_partition_constr = false;

This looks better (it will avoid unnecessary ExecConstraints() call) :

if (resultRelInfo->ri_PartitionRoot == NULL ||
(resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_insert_before_row))
check_partition_constr = resultRelInfo->ri_PartitionCheck;
else
check_partition_constr = false;

>
> --
> Thanks,
> -Amit Khandekar
> EnterpriseDB Corporation
> The Postgres Database Company

--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company