Re: Assert failure in try_nestloop_path()

Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> Yes. I've merged the clone-dedup fix and your
> drop-redundant-equivalence-clauses fix into a single patch (0001
> attached), removed the find_clause_by_serial cross-checks, and
> un-commented the assertion in get_joinrel_parampathinfo, which now
> holds with your fix in place.

The one nit I can find to pick with this is that I don't really like
the description of the clauses of concern as "textually identical".
That makes it sound like surface syntax details (like number of
spaces or presence of an unnecessary table qualifier) matter.
Perhaps say "parse-tree identical", or "equal()"? That seems a bit
jargony but I have no better ideas.

> I've kept the create_join_clause change as a separate patch (0002),
> since it's a distinct issue. There the trouble is that two
> RestrictInfos for the same condition end up with different
> rinfo_serials, rather than that we enforce multiple clones of the
> same clause.

I wonder if we are going to push the rinfo_serial mechanism too far.
But that's just a vague feeling of discomfort, I can't say there is
anything wrong with fixing it this way.

> I plan to push both to HEAD and v19 but no further if there are no
> objections.

I wonder if we should leave the Assert bits out of v19. The intent to
have hard prevention of duplicate clauses is a brand new aspiration,
and I have pretty much no faith that no such cases remain. I don't
really want to commit to fixing all such cases in v19.

regards, tom lane