Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> writes: > On Tue, Apr 11, 2017 at 3:53 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > + * there is no need for EPQ recheck at a join (and Vars or Aggrefs in > + * the qual might not be available locally anyway).
> I am not sure whether EPQ checks make sense for an upper relation, esp. a > grouped relation. So mentioning Aggref can be confusing here. For joins, we > execute EPQ by executing the (so called) outer plan created from > fdw_outerpath. > For this, we fetch whole-row references for the joining relations and build > the > output row by executing the local outer plan attached to the ForeignScanPlan. > This whole-row references has values for all Vars, so even though Vars are not > available, corresponding column values are available. So mentioning Vars is > also confusing here. Well, my first attempt at fixing this merged remote_conds and remote_exprs together, which in the previous version of the code resulted in always passing the remote conditions as fdw_recheck_quals too. And what happened was that I got "variable not found in subplan target list" errors for Vars used inside Aggrefs in pushed-to-the-remote HAVING clauses. Which is unsurprising -- it'd be impossible to return such a Var if the grouping is being done remotely. So I think it's important for this comment to explain that we *can't* put upperrel quals into fdw_recheck_quals, not just that "there's no need to". But pointing out that at a join, there's a different mechanism that's responsible for EPQ checks is good. I'll reword this again. > We can club the code to separate other and join clauses, checking that all > join > clauses are shippable and separating other clauses into local and remote > clauses in a single list traversal as done in the attached patch. OK. I was trying to be noninvasive, but this does look more sensible. I think this might read better if we inverted the test (so that the outer-join-joinclause-must-be-pushable case would come first). If we're going for a more-than-minimal patch, I'm inclined to also move the first loop in postgresGetForeignPlan into the "else" branch, so that it doesn't get executed in the join/upperrel case. I realize that it's going to iterate zero times in that case, but it's just confusing to have it there when we don't expect it to do anything and we would only throw away the results if it did do anything. regards, tom lane -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers