Re: BUG #17781: Assert in setrefs.c

Richard Guo <guofenglinux@gmail.com> writes:
> Hmm. I begin to wonder if it's better to use phinfo->ph_eval_at instead
> in build_joinrel_tlist when we check whether the PHV actually comes from
> the nullable side of an outer join.

I wondered about that too, but I think phrels is probably preferable,
because it is fundamentally a syntactic attribute and what we are
after here is matching the syntactic structure.  I could be convinced
that that's wrong, but I'd want to see an actual counterexample.

> Even so it seems we still need to update phv->phrels in
> remove_rel_from_query when we remove a left join.  Otherwise it'd be
> weird to observe that phrels contains some already-removed relids.

Agreed, that's a good catch.  As the comment for remove_rel_from_query
says,

 * We are not terribly thorough here.

It's not surprising that the requirements for it advance over time.

(At one point I wrote some code that taught it to clean out the
removed relids from EquivalenceClasses, which doesn't need
to happen right now because we don't ever put outer-join quals
into EquivalenceClasses.  I don't think I included that in any
of the published versions of the patch series, but it's going to
need to reappear eventually.)

            regards, tom lane