Re: Problem with default partition pruning

On 2019-Jul-03, Amit Langote wrote:

> Hosoya-san,
> 
> Thanks for updating the patches.
> 
> I have no comment in particular about
> v4_default_partition_pruning.patch,

Cool, thanks.  I spent some time reviewing this patch (the first one)
and I propose the attached cosmetic changes.  Mostly they consist of a
few comment rewordings.

There is one Assert() that changed in a pretty significant way ...
innocent though the change looks.  The original (not Hosoya-san's
patch's fault) had an assertion which is being changed thus:

    minoff = 0;
    maxoff = boundinfo->ndatums;
    ...
        if (partindices[minoff] < 0)
            minoff++;
        if (partindices[maxoff] < 0)
            maxoff--;
 
        result->scan_default = partition_bound_has_default(boundinfo);
-       Assert(minoff >= 0 && maxoff >= 0);
+       Assert(partindices[minoff] >= 0 &&
+              partindices[maxoff] >= 0);

Note that the original Assert() here was verifying whether minoff and
maxoff are both >= 0.  But that seems pretty useless since it seems
almost impossible to have them become that given what we do to them.
What I think this code *really* wants to check is whether *the partition
indexes* that they point to are not negative: the transformation that
the two "if" lines do means to ignore bounds that correspond to value
ranges uncovered by any partition.  And after the incr/decr operators,
we expect that the bounds will be those of existing partitions ... so
they shouldn't be -1.


Other changes are addition of braces to some one-line blocks that had
significant comments, and minor code rearrangements to make things look
more easily understandable.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services