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>, Alan Jackson <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-17 21:30:11
Message-ID: 14101.1558128611@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/17 7:50, Tom Lane wrote:
>> Also, it seems like we could save at least some of the planning expense by
>> having the PARTCLAUSE_INITIAL pass report back whether it had discarded
>> any Param-bearing clauses or not. If not, there's definitely no need to
>> run the PARTCLAUSE_EXEC pass.

> That sounds like a good idea.

Here's a patch that does it like that. Since this requires additional
output values from gen_partprune_steps, I decided to go "all in" on
having GeneratePruningStepsContext contain everything of interest,
including all these result flags. That makes the patch a bit more
invasive than it could perhaps be otherwise, but I think it's cleaner.
And these are just internal APIs in partprune.c so there's no
compatibility problems to worry about.

It's kind of hard to tell what this code is actually doing, due to
the lack of visibility in EXPLAIN output (something I think Robert
has kvetched about before). To try to check that, I made the very
quick-and-dirty explain.c patches below, which I'm *not* proposing
to commit. They showed that this code is making the same decisions
as HEAD for all the currently-existing test cases, which is probably
what we want it to do. The new test cases added by the main patch
show up as having different initial and exec step lists, which is
also expected. So I feel pretty good that this is acting as we wish.

The fly in the ointment is what about back-patching? After looking
around, I think that we can probably get away with changing the run-time
data structures (PartitionPruneContext, PartitionedRelPruningData,
PartitionPruningData) in v11; it seems unlikely that anything outside
partprune.c and execPartition.c would have occasion to touch those.
But it's harder to argue that for the node type PartitionedRelPruneInfo,
since that's part of plan trees. What I suggest we do for v11 is to
leave the existing fields in that node type that're removed by this
patch present, but unused, and add initial_pruning_steps and
exec_pruning_steps at the end. This would confuse any code that's
studying the contents of PartitionedRelPruneInfo closely, but
probably as long as we set do_initial_prune and do_exec_prune properly,
that would satisfy most code that might be inspecting it.

Since time is growing short before beta1 wrap, I'm going to go ahead
and push this in hopes of getting buildfarm cycles on it. But feel
free to review and look for further improvements. There's plenty
of time to rethink the data-structure details for the v11 backpatch,
also.

regards, tom lane

Attachment Content-Type Size
refactor-pruning-step-creation-1.patch text/x-diff 60.2 KB
explain-hack.patch text/x-diff 2.6 KB
explain-hack-head.patch text/x-diff 2.6 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Andrew Gierth 2019-05-17 21:34:28 Re: BUG #15812: Select statement of a very big number, with a division operator seems to round up.
Previous Message Andres Freund 2019-05-17 20:24:43 Re: BUG #15804: Assertion failure when using logging_collector with EXEC_BACKEND