Re: speeding up planning with partitions

On 2019/02/21 0:50, Tom Lane wrote:
> However, there are other reasons why I'm not really happy with the
> approach proposed in this patch.
> 
> The main problem is that cloning the PlannerInfo while still sharing a lot
> of infrastructure between the clones is a horrid hack that I think will be
> very buggy and unmaintainable.  We've gotten away with it so far in
> inheritance_planner because (a) the complexity is all local to that
> function and (b) the cloning happens very early in the planning process,
> so that there's not much shared subsidiary data to worry about; mostly
> just the parse tree, which in fact isn't shared because the first thing
> we do is push it through adjust_appendrel_attrs.  This patch proposes
> to clone much later, and both the actual cloning and the consequences
> of that are spread all over, and I don't think we're nearly done with
> the consequences :-(.  I found the parameterized-path problem while
> wondering why it was okay to not worry about adjusting attrs in data
> structures used during path construction for other baserels ... turns
> out it isn't.  There's a lot of other stuff in PlannerInfo that you're
> not touching, for instance pathkeys and placeholders; and I'm afraid
> much of that represents either current bugs or future problems.
> 
> So what I feel we should do is set this aside for now and see if we
> can make something of the other idea I proposed.  If we could get
> rid of expand-inheritance-at-the-top altogether, and plan inherited
> UPDATE/DELETE the same as inherited SELECT, that would be a large
> reduction in planner complexity, hence much to be preferred over this
> approach which is a large increase in planner complexity.  If that
> approach crashes and burns, we can come back to this.

OK, I agree that the other approach might be a better way forward.  It'll
not just improve the performance in an elegant manner, but will also make
other projects more feasible, such as, MERGE, what Fujita-san mentioned on
the other thread, etc.

> There might be parts of this work we can salvage, though.  It seems
> like the idea of postponing expand_inherited_tables() might be
> something we could use anyway.

+1.  So, let's try to do things in this order:

1. Make inheritance-expansion-at-bottom case perform better now,
addressing at least SELECT performance in PG 12, provided we manage to get
the patches in order in time (I'll try to post the updated
lazy-inheritance-expansion patch later this week.)

2. Overhaul inherited UPDATE/DELETE planning to use
inheritance-expansion-at-bottom (PG 13)

It's unfortunate that UPDATE/DELETE won't perform as well as SELECTs even
couple of releases after declarative partitioning was introduced, but I
agree that we should solve the underlying issues in an elegant way.

Thanks,
Amit