Re: inconsistent results querying table partitioned by date

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, ajax(at)tvsquared(dot)com, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: inconsistent results querying table partitioned by date
Date: 2019-05-14 20:58:20
Message-ID: 28453.1557867500@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> On 2019/05/14 15:09, David Rowley wrote:
>> Do you think it's a bad idea to do it this way?

> As I mentioned above, I think this may be quite a bit of code churn for
> enforcing a policy that's elsewhere enforced in a much simpler manner.
> How about deferring this to committer?

Not sure I'll be the one to commit this, but ...

I don't much like the approach in
v1-0003-Fix-planner-to-not-prune-partitions-with-non-immu.patch
because (a) it appears to me to add duplicative expression mutability
testing, which is kind of expensive since that adds syscache lookups,
plus it adds such tests even for clauses that aren't going to match any
partition keys; and (b) it's extremely nonobvious that this restricts
plan-time pruning and not run-time pruning. I do see that
prune_append_rel_partitions is used only for the former; but you sure
as heck can't deduce that from any of its comments, so somebody might
try to use it for run-time pruning.

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?

(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.)

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?

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2019-05-14 21:30:57 Re: BUG #15804: Assertion failure when using logging_collector with EXEC_BACKEND
Previous Message Tom Lane 2019-05-14 19:52:04 Re: BUG #15804: Assertion failure when using logging_collector with EXEC_BACKEND