Re: wrong query result due to wang plan - Mailing list pgsql-hackers

Richard Guo <guofenglinux@gmail.com> writes:
> It occurs to me that we can leverage JoinDomain to tell if we are below
> the nullable side of a higher-level outer join if the clause is not an
> outer-join clause.  If we are belew outer join, the current JoinDomain
> is supposed to be a proper subset of top JoinDomain.  Otherwise the
> current JoinDomain must be equal to top JoinDomain.  And that leads to a
> fix as code changes below.

That doesn't look right at all: surely this situation can occur further
down in a join tree, not only just below top level.

I thought about this some more and realized that the whole business of
trying to push a qual up the join tree is probably obsolete.

In the first place, there's more than one problem with assigning the
ON FALSE condition the required_relids {t2, t3, t4, t2/t4}.  As you say,
it causes bogus conclusions about which joinrel can be considered empty if
we commute the outer joins' order.  But also, doing it this way loses the
information that t2/t3 can be considered empty, if we do the joins in
an order where that is useful to know.

In the second place, I think recording the info that t2/t3 is empty is
probably sufficient now, because of the mark_dummy_rel/is_dummy_rel
bookkeeping in joinrels.c (which did not exist when we first added this
"push to top of tree" hack).  If we know t2/t3 is empty then we will
propagate that knowledge to {t2, t3, t4, t2/t4} when it's formed,
without needing to have a clause that can be applied at that join level.

So that leads to a conclusion that we could just forget the whole
thing and always use the syntactic qualscope here.  I tried that
and it doesn't break any existing regression tests, which admittedly
doesn't prove a lot in this area :-(.  However, we'd then need to
decide what to do in process_implied_equality, which is annotated as

 * "qualscope" is the nominal syntactic level to impute to the restrictinfo.
 * This must contain at least all the rels used in the expressions, but it
 * is used only to set the qual application level when both exprs are
 * variable-free.  (Hence, it should usually match the join domain in which
 * the clause applies.)

and indeed equivclass.c is passing a join domain relid set.  It's
not hard to demonstrate that that path is also broken, for instance

explain (costs off)
select * from int8_tbl t1 left join
  (int8_tbl t2 inner join int8_tbl t3 on (t2.q1-t3.q2) = 0 and (t2.q1-t3.q2) = 1
   left join int8_tbl t4 on t2.q2 = t4.q2)
on t1.q1 = t2.q1;

One idea I played with is that we could take the join domain relid set
and subtract off the OJ relid and RHS relids of any fully-contained
SpecialJoinInfos, reasoning that we can reconstruct the fact that
those won't make the join domain's overall result nondummy, and
thereby avoiding the possibility of confusion if we end up commuting
with one of those joins.  This feels perhaps overly complicated,
though, compared to the brain-dead-simple "use the syntactic scope"
approach.  Maybe we can get equivclass.c to do something equivalent
to that?  Or maybe we only need to use this complex rule in
process_implied_equality?

(I'm also starting to realize that the current basically-syntactic
definition of join domains isn't really going to work with commutable
outer joins, so maybe the ultimate outcome is that the join domain
itself is defined in this more narrowly scoped way.  But that feels
like a task to tackle later.)

            regards, tom lane