Re: FailedAssertion on partprune

On 2018-Aug-13, Robert Haas wrote:

> In my original design, apply_scanjoin_target_to_paths() -- or whatever
> I called it in the original patch versions -- built an entirely new
> RelOptInfo, so that we ended up with one RelOptInfo representing the
> unvarnished result of scan/join planning, and a second one
> representing the result of applying the scan/join target to that
> result. However, Amit Kapila was able to demonstrate that this caused
> a small but noticeable regression in planning speed, which seemed like
> it might plausibly cause some issues for people running very simple
> queries.
>
> In that original design, if the topmost scan/join rel was partitioned,
> the new "tlist" upper rel was also partitioned, and in the same way.
> In short, this was a kind of "partitionwise tlist" feature. For each
> child of the topmost scan/join rel, we built a child "tlist" rel,
> which got the same paths but with the correct tlist applied to each
> one. The path for the toplevel "tlist" upper rel could then be built
> by appending the children from each child "tlist" rel, or else by the
> usual method of sticking a Result node over the cheapest path for the
> topmost scan/join rel. In general, the first method is superior.
> Instead of a plan like Result -> Append -> (N children each producing
> the unprojected tlist), you get Append -> (N children each producing
> the projected tlist), which is cheaper.

I have to admit I have a hard time understanding this stuff. I don't
know what a "scan/join target" is (or scan/join relation for that
matter) ... and this "tlist relation" thing is an entirely new concept
to me.

I agree that pushing down the projection looks worthwhile.

> To avoid the performance overhead of creating a whole new tree of
> upper rels, I rewrote the code so that it modifies the RelOptInfos in
> place. That unfortunately makes the logic a bit more confusing, and
> it sounds from your remarks like I didn't comment it as clearly as
> might have been possible. But the basic idea is the same: we want the
> projection to be passed down to the child nodes rather than getting a
> Result node on top. The commit that did most of this --
> 11cf92f6e2e13c0a6e3f98be3e629e6bd90b74d5 -- also solved a few other
> problems, as noted in the commit message, so I don't think we want to
> rip it out wholesale. There might be better ways to do some of it,
> though.

So,
1. I'm not sure that partition pruning devs are on the hook for
producing a test case for the problem originally reported,
since we're clearly dealing with a wacko plan,
2. I wonder if this should be considered an open item related to commit
11cf92f6e2e1 instead of partprune.

The other option is to ignore this whole affair, mark this as solved in
the open items list, and move on.

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