On Mon, Feb 20, 2023 at 4:56 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > * I suspect the other use of get_common_eclass_indexes, in > have_relevant_eclass_joinclause, is broken as well.
It seems have_relevant_joinclause is broken for the presented case. It does not get a change to call have_relevant_eclass_joinclause, because flag 'has_eclass_joins' is not set for t1 due to t1 being not in the EC's ec_relids. As a result, have_relevant_joinclause thinks there is no joinclause that involves t1 and t2, which is not right. > * This fix throws away a fair bit of the optimization intended by > 3373c7155, since it will result in examining some irrelevant ECs. > I'm not sure if it's worth complicating get_common_eclass_indexes > to try to recover that by adding knowledge about outer joins. Yeah, this is also my concern that we'd lose some optimization about finding ECs. > * I'm now kind of wondering whether there are pre-existing bugs of the > same ilk. Maybe not, because before 2489d76c4 an EC constraint that was > computable at the join but not earlier would have to have mentioned both > sides of the join ... but I'm not quite sure. I also think there is no problem before, because if a clause was computable at the join but not earlier and only mentioned one side of the join, then it was a non-degenerate outer join qual or an outerjoin_delayed qual, and cannot enter into EC. > BTW, while looking at this I saw that generate_join_implied_equalities' > calculation of nominal_join_relids is wrong for child rels, because it > fails to fold the join relid into that if appropriate. I dug a little into this and it seems this is all right as-is. Among all the calls of generate_join_implied_equalities, it seems only build_joinrel_restrictlist would have outer join's ojrelid in param 'join_relids'. And build_joinrel_restrictlist does not get called for child rels. The restrictlist of a child rel is constructed from that of its parent rel. Thanks Richard