Sorry for delayed response.

On Sun, Oct 21, 2012 at 3:16 AM, Kohei KaiGai <kai...@kaigai.gr.jp> wrote:
2012/10/11 Etsuro Fujita <fujita.ets...@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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to