On 2016/11/11 19:22, Ashutosh Bapat wrote:
The patch looks in good shape now.
Thanks for the review!
The patch looks in good shape now. Here are some comments. I have also
made several changes to comments correcting grammar, typos, style and
at few places logic. Let me know if the patch looks good.
OK, will look into that.
I guess, below code
+ if (!fpinfo->subquery_rels)
+ return false;
can be changed to
if (!bms_is_member(node->varno, fpinfo->subquery_rels))
return false;
Also the return values from the recursive calls to isSubqueryExpr() can be
returned as is. I have included this change in the patch.
Will look into that too.
deparse.c seems to be using capitalized names for function which
actually deparse something and an non-capitalized form for helper
functions.
That's not true. There is a function named classifyConditions(). The
function naming in deparse.c is a bit arbitrary.
From that perspective attached patch renames isSubqueryExpr
as is_subquery_var() and getSubselectAliasInfo() as
get_alias_id_for_var(). Actually both these functions accept a Var
node but somehow their names refer to expr.
The reason why I named that function isSubqueryExpr is that I think that
function would be soon extended so as to handle PHVs. See another patch
for evaluating PHVs remotely.
This patch is using make_tlist_from_pathtarget() to create tlist to be
deparsed but search in RelOptInfo::reltarget::exprs for a given Var.
As long as the relations deparsed do not carry expressions, this might
work, but it will certainly break once we start deparsing relations
with expressions since the upper most query's tlist contains only
Vars. Instead, we should probably, create tlist and save it in fpinfo
and use it later for searching (tlist_member()?). Possibly use using
build_tlist_to_deparse(), to create the tlist similar so that
targetlist list creation logic is same for all the relations being
deparsed. I haven't included this change in the patch.
Seems like a good idea. Will revise.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
