On 2016/11/24 18:20, Ashutosh Bapat wrote:
I wrote:
>>>> You missed the point; the foreignrel->reltarget->exprs doesn't contain
>>>> any
>>>> PHVs, so the tlist created by build_tlist_to_depase will be guaranteed to
>>>> be
>>>> one-to-one with the foreignrel->reltarget->exprs.
You wrote:
>>> It's guaranteed now, but can not be forever. This means anybody who
>>> supports PHV tomorrow, will have to "remember" to update this code as
>>> well. If s/he misses it, a bug will be introduced. That's the avenue
>>> for bug, I am talking about.
I wrote:
>> It *can* be guaranteed. See another patch for supporting evaluating PHVs
>> remotely.
> We can't base assumptions in this patch on something in the other
> patch, esp. when that's not even reviewed once. PHV is just one case,
> subqueries involving aggregates is other and there are others.
Let's discuss this together with the patch for PHVs. That was one of
the reasons why I had merged the two patches into one. I'd like to
leave enhancements such as subqueries involving aggregates for future
work, though.
> But
> that's not really the point. The point is
> add_to_flat_tlist(pull_var_clause(rel->reltarget->exprs) can not be
> proved to be same as rel->reltarget->exprs in general.
Really? As mentioned in comments for RelOptInfo in relation.h shown
below, the rel->reltarget->exprs for a base/join relation only contains
Vars and PHVs, so the two things would be proved to be one-to-one.
(Note: we don't need to care about the appendrel-child-relation case
because appendrel child relations wouldn't appear in the main join tree.)
* reltarget - Default Path output tlist for this rel; normally
contains * Var and PlaceHolderVar nodes for the values we need to * output from this
relation.* List is in no particular order, but all rels of an * appendrel set must
usecorresponding orders. * NOTE: in an appendrel child relation, may contain *
arbitraryexpressions pulled up from a subquery!
Best regards,
Etsuro Fujita
