On Mon, Dec 5, 2022 at 8:13 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Ashutosh Bapat <ashutosh.bapat....@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