Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> On Thu, Jan 12, 2017 at 6:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think you'd just end up putting it back at some point. It's the only
>> means that foreign_expr_walker() has for getting at the root pointer,
>> and nearly all planner code needs that. Seems to me it's just chance
>> that foreign_expr_walker() doesn't need it right now.
> I agree with you that most of the planner code needs that but not
> every planner function accepts root as an argument.
[ shrug... ] The general trend in the planner is that things get more
complicated because we start accounting for new effects. I don't have
any faith in the claim that because is_foreign_expr hasn't needed
PlannerInfo yet, it never will, because what it's required to do is
inherently spongy and complicated. It wouldn't particularly surprise
me if someone wanted to start putting cost considerations in there,
for example.
In short, I think that removing this argument is just make-work that
we're very likely to have to undo at some point; and the further you
extend the changes, the larger a hazard for back-patching it will be.
If you can convince some other committer that this is a good idea, fine,
but I won't commit it.
regards, tom lane
