Re: FailedAssertion on partprune

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: FailedAssertion on partprune
Date: 2018-08-03 00:39:59
Message-ID: CAKJS1f81T8_nhTRLDoUgPxyJFJjU3YCjXiV9EYWXGUPODCDuqw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 3 August 2018 at 05:54, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Yeah, looking at the explain posted upthread, the issue is specifically
> that some of the child paths might be for Gathers over subsets of the
> partitioning hierarchy. It's not real clear to me whether such a subset
> would necessarily correspond to a subtree of the hierarchy, but if it
> does, you'd want to be able to apply run-time pruning at the parent
> Append, in hopes of not having to execute the Gather at all. Separately
> from that, if you do execute the Gather, applying runtime pruning within
> its child Append is also a good thing.

Yeah, that "good thing" was the reason I changed my mind from aborting
run-time pruning in my first patch idea to allowing it because if
pruning says so, it's fine to remove an entire sub-partitioned
hierarchy.

If we look at the Assert that's in question.

/* Same rel cannot be both leaf and non-leaf */
Assert(relid_subplan_map[rti] == 0);

If this fails then the subplan path that was found must have a parent
that's a partitioned table. If we're worried about those containing
subplans which are not partitions then we can probably add some
Asserts somewhere else.

One place that we could do that with a bit of cover would be inside
the bms_num_members(allmatchedsubplans) < list_length(subpaths)
condition, we could do:

Assert(root->simple_rte_array[parentrel->relid]->relkind !=
RELKIND_PARTITIONED_TABLE);

There should never be any other_subplans when the parent is a partitioned table.

I've attached a patch to demonstrate what I mean here.

> So what's unclear to me is whether removing that Assert is sufficient
> to make all of that work nicely. I'm concerned in particular that the
> planner might get confused about which pruning tests to apply at each
> level. (EXPLAIN isn't a very adequate tool for testing this, since
> it fails to show anything about what pruning tests are attached to an
> Append. I wonder if we need to create something that does show that.)

Pruning is only ever applied at the top-level when the RelOptInfo is a
RELOPT_BASEREL. It might well be that we want to reconsider that now
that we've seen some plans where the subpaths are not always pulled
into the top-level [Merge]Append path, but as of now I don't quite see
how this could get broken. We're just pruning with a coarser
granularity.

> I think it'd be a real good idea to have a test case to exercise this.
> Jaime's presented test case is too expensive to consider as a regression
> test, but I bet that a skinnied-down version could be made to work
> once you'd set all the parallelization parameters to liberal values,
> like in select_parallel.sql.

Yeah, I did spend a bit of time trying to cut down that test and
failed at it. The fact that it seemed so hard to do that didn't give
me much confidence that the plan produced by the test would be very
stable, but perhaps we can just deal with that if we see its unstable.

I'll try again with the test.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
run-time_prune_asserts_for_discussion.patch application/octet-stream 1.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-08-03 00:42:21 Re: FailedAssertion on partprune
Previous Message Andrew Gierth 2018-08-02 23:31:21 Re: Should contrib modules install .h files?