Richard Guo <guofenglinux@gmail.com> writes: > Considering that clone clauses should always be outer-join clauses not > filter clauses, I'm wondering if we can add an additional check for that > in RINFO_IS_PUSHED_DOWN, something like
I don't like that one bit; it seems way too likely to introduce bad side-effects elsewhere.
Agreed. I also do not have too much confidence in it.
I think the real issue is that "is pushed down" has never been a conceptually accurate way of thinking about what remove_rel_from_query needs to do here. Using RINFO_IS_PUSHED_DOWN happened to work up to now, but it's an abuse of that macro, and changing the macro's behavior isn't the right way to fix it.
Having said that, I'm not sure what is a better way to think about it. It seems like our data structure doesn't have a clear tie between restrictinfos and their original join level, or at least, to the extent that it did the "clone clause" mechanism has broken it.
I wonder if we could do something involving recording the rinfo_serial numbers of all the clauses extracted from a particular syntactic join level (we could keep this in a bitmapset attached to each SpecialJoinInfo, perhaps) and then filtering the joinclauses on the basis of serial numbers instead of required_relids.
I think this is a better way to fix the issue. I went ahead and drafted a patch as attached. But I doubt that the collecting of rinfo_serial numbers is thorough in the patch. Should we also collect the rinfo_serial of quals generated in reconsider_outer_join_clauses()? I believe that we do not need to consider the quals from generate_base_implied_equalities(), since they are all supposed to be restriction clauses not join clauses.
