Re: speeding up planning with partitions

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