Sorry for delayed response.
On Sun, Oct 21, 2012 at 3:16 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
> 2012/10/11 Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>:
>> I've reviewed your patch quickly. I noticed that the patch has been created in
>> a slightly different way from the guidelines:
>> http://www.postgresql.org/docs/devel/static/fdw-planning.html The guidelines
>> say the following, but the patch identifies the clauses in
>> baserel->baserestrictinfo in GetForeignRelSize, not GetForeignPaths. Also, it
>> has been implemented so that most sub_expressions are evaluated at the remote
>> end, not the local end, though I'm missing something. For postgresql_fdw to be
>> a good reference for FDW developers, ISTM it would be better that it be
>> consistent with the guidelines. I think it would be needed to update the
>> following document or redesign the function to be consistent with the following
>> document.
>>
> Hmm. It seems to me Fujita-san's comment is right.
Indeed postgresql_fdw touches baserestrictinfo in GetForeignRelSize, but
it's because of optimization for better width estimate. The doc
Fujita-san pointed says that:
> The actual identification of such a clause should happen during
> GetForeignPaths, since it would affect the cost estimate for the
> path.
I understood this description says that "you need to touch baserestrict
info *before* GetForeignPlan to estimate costs of optimized path". I
don't feel that this description prohibits FDW to touch baserestrictinfo
in GetForeignRelSize, but mentioning it clearly might be better.
Regards,
--
Shigeru HANADA
