On Mon, Dec 5, 2022 at 8:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
> > partition-wise join should be willing to fallback to non-partitionwise
> > join in such a case. After spending a few minutes with the code, I
> > think generate_partitionwise_join_paths() should not call
> > set_cheapest() is the pathlist of the child is NULL and should just
> > wind up and avoid adding any path.
>
> We clearly need to not call set_cheapest(), but that's not sufficient;
> we still fail at higher levels, as you'll see if you try the example
> Richard found. I ended up making fe12f2f8f to fix this.
Thanks. That looks good.
>
> I don't especially like "rel->nparts = 0" as a way of disabling
> partitionwise join; ISTM it'd be clearer and more flexible to reset
> consider_partitionwise_join instead of destroying the data structure.
> But that's the way it's being done elsewhere, and I didn't want to
> tamper with it in a bug fix. I see various assertions about parent
> and child consider_partitionwise_join flags being equal, which we
> might have to revisit if we try to make it work that way.
>
AFAIR, consider_partitionwise_join tells whether a given partitioned
relation (join, higher or base) can be considered for partitionwise
join. set_append_rel_size() decides that based on some properties. But
rel->nparts is indicator of whether the relation (join, higher or
base) is partitioned or not. If we can not generate AppendPath for a
join relation, it means there is no way to compute child join
relations and thus the relation is not partitioned. So setting
rel->nparts = 0 is right. Probably we should add macros similar to
dummy relation for marking and checking partitioned relation. I see
IS_PARTITIONED_RELATION() is defined already. Maybe we could add
mark_(un)partitioned_rel().
--
Best Wishes,
Ashutosh Bapat
