Re: inconsistent results querying table partitioned by date

Thanks for the updated patches.

On 2019/05/15 8:52, David Rowley wrote:
> On Wed, 15 May 2019 at 08:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So I think David's got the right idea that match_clause_to_partition_key
>> is where to handle this, and I like the fact that his patch explicitly
>> represents whether we're trying to do run-time or plan-time pruning.
>> I agree it's kind of invasive, and I wonder why.  Shouldn't the
>> additional flag just be a field in the "context" struct, instead of
>> being a separate parameter that has to be laboriously passed through
>> many call levels?
> 
> Thanks for having a look.  I originally stayed clear of "context"
> since it looks more like how we output the pruning steps, rather than
> a context as to how they should be generated. But it's true, using it
> saves having to pass the extra argument around, so I made that change.
> 
>> (For myself, I'd have made it just a bool not an enum, given that
>> I don't foresee a need for additional values.  But that's definitely
>> a matter of preference.)
> 
> I've changed to bool, mostly because it makes the patch a bit smaller.

Looks good.  Thanks for tweaking many comments for clarity, except this
one could be improved a bit more:

+ * 'forplanner' must be true when being called from the query planner.

This description of 'forplanner' might make it sound like it's redundant
(always true), because gen_partprune_steps() is only called from the
planner today.  How about making it say the same thing as the comment next
to its definition in GeneratePruningStepsContext struct):

'forplanner' must be true if generating steps to be used during query
planning.

>> Also, I'm a bit confused by the fact that we seem to have a bunch
>> of spare-parts patches in this thread.  What combination is actually
>> being proposed for commit?
> 
> I was also confused at the extra two patches Amit posted. The extra
> one for the tests didn't look very valid to me, as I mentioned
> yesterday.

As I said in my previous reply, 0001 contains wrong expected output
because of the other bugs discussed earlier.  I guess that's not very helpful.

Anyway, I'm attaching a squashed version of those two patches as
0001-Fix-bugs-in-pruning-with-composite-range-partition-k.patch.  It
applies as-is to both master and v11 branches.

> I propose the attached for master, and I'll post something for v11 shortly.

Thanks.  I'm re-attaching both of your patches here just to have all the
patches in one place.

Thanks,
Amit