Re: remove_useless_joins vs. bug #19560

Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> I looked at the patch. I noticed a couple of problems with putting
> the preprocessed expression back into the subquery.

Thanks for spotting these issues.

> One is that preprocess_expression is not something we can run twice on
> the same expression.

Hmm, is that really true? I'm moderately certain that there are
existing code paths where that can happen. I see your point about
hitting !IsA(node, SubPlan) assertions, but I wonder if that's
connected to the unpleasantly incestuous relationship between
SS_process_sublinks and SS_replace_correlation_vars. That line of
thinking probably leads to the same endpoint though, namely that
we want to do this somewhere else.

I'll study this some more and look for additional holes in it.

> One consequence is that a not-yet-planned subquery can now contain
> SubPlans, within a PHV of an upper query level. (This applies to your
> patch as well.) This seems to be new. AFAICT, an unplanned subquery
> never held SubPlans before. Fortunately, most of the tree walkers
> that run on the subquery before SS_replace_correlation_vars replaces
> the outer PHV with a Param cope with that. The exception is
> flatten_join_alias_vars, which descends into every PHV and asserts on
> SubPlans. I changed it to leave PHVs of upper levels alone; such a
> PHV cannot contain join aliases of the level being flattened anyway.

I wonder if we should try to run SS_replace_correlation_vars earlier.
That's rather a scary change to be contemplating for stable branches
though, precisely because of these hard-to-foresee interactions.

regards, tom lane