Re: Wrong query results caused by loss of join quals

On Mon, Feb 20, 2023 at 4:56 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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