Horton

Re: speeding up planning with partitions

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

View thread View on postgr.es
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Tom Lane <tgl@sss.pgh.pa.us>
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-20T05:47:15Z
Lists: pgsql-hackers 
On 2019/02/20 5:57, Tom Lane wrote:
> 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.

Hmm, yeah.  It wouldn't have crashed with an earlier version of the patch,
because with it, we were setting the parent relation's rows to a dummy
value of 1 at the end of set_inherited_target_rel_sizes, which I removed
in the latest patch after your comment upthread.

> 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),

Actually, reparameterize_path_by_child (invented by partitionwise-join
commit) seems to take care of fixing up the Path to have child attributes,
so the plan comes out exactly as on HEAD.  But to be honest, that means
this new approach of inherited update join planning only appears to work
by accident.

> 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.

What if we set the parent target relation's rows to an average of child
target relation's rows, that is, instead of setting it to dummy 1 that
previous versions of the patches were doing?

Thanks,
Amit

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.