Horton

Re: speeding up planning with partitions

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: David Rowley <david.rowley@2ndquadrant.com>, "Imai, Yoshikazu" <imai.yoshikazu@jp.fujitsu.com>, Amit Langote <amitlangote09@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2019-02-19T20:57:53Z
Lists: pgsql-hackers 
I wrote:
> OK, I'll make another pass over 0001 today.

So I started the day with high hopes for this, but the more I looked at
it the less happy I got, and finally I ran into something that looks to
be a complete show-stopper.  Namely, that the patch does not account
for the possibility of an inherited target rel being the outside for a
parameterized path to some other rel.  Like this example in the
regression database:

explain update a set aa = aa + 1
from tenk1 t where a.aa = t.unique2;

With HEAD, you get a perfectly nice plan that consists of an append
of per-child plans like this:

   ->  Nested Loop  (cost=0.29..8.31 rows=1 width=16)
         ->  Seq Scan on a  (cost=0.00..0.00 rows=1 width=10)
         ->  Index Scan using tenk1_unique2 on tenk1 t  (cost=0.29..8.30 rows=1 
width=10)
               Index Cond: (unique2 = a.aa)

With the 0001 patch, this gets an Assert during set_base_rel_pathlists,
because indxpath.c tries to make a parameterized path for tenk1
with "a" as the outer rel.  Since tenk1's joinlist hasn't been
touched, it's still referencing the inheritance parent, and the
code notices that we haven't made a rowcount estimate for that.
Even if we had, we'd generate a Path referencing Vars of the parent
rel, which would not work.

Conceivably, such a Path could be fixed up later (say by applying
adjust_appendrel_attrs to it during createplan.c), but that is not
going to fix the fundamental problem: the cost estimate for such a
Path should vary depending on how large we think the outer rel is,
and we don't have a reasonable way to set that if we're trying to
make a one-size-fits-all Path for something that's being joined to
an inheritance tree with a widely varying set of relation sizes.

So I do not see any way to make this approach work without a
significant(?) sacrifice in the quality of plans.

I've got other issues with the patch too, but it's probably not
worth getting into them unless we can answer this objection.

			regards, tom lane

Commits

  1. Clean up handling of constraint_exclusion and enable_partition_pruning.

  2. Add test case exercising formerly-unreached code in inheritance_planner.

  3. Speed up planning when partitions can be pruned at plan time.

  4. Avoid crash in partitionwise join planning under GEQO.

  5. Avoid passing query tlist around separately from root->processed_tlist.

  6. Build "other rels" of appendrel baserels in a separate step.

  7. Get rid of duplicate child RTE for a partitioned table.

  8. Rearrange make_partitionedrel_pruneinfo to avoid work when we can't prune.

  9. Don't copy PartitionBoundInfo in set_relation_partition_info.

  10. Move building of child base quals out into a new function

  11. Reorganize planner code moved in b60c39759908

  12. Move inheritance expansion code into its own file

  13. Fix inherited UPDATE/DELETE with UNION ALL subqueries.

  14. Rearrange planner to save the whole PlannerInfo (subroot) for a subquery.