Re: postgres_fdw behaves oddly

(2014/11/18 18:27), Ashutosh Bapat wrote:
> On Tue, Nov 18, 2014 at 1:50 PM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp <mailto:fujita.etsuro@lab.ntt.co.jp>> wrote:
>     (2014/11/17 19:36), Ashutosh Bapat wrote:

>         Here are my comments about the patch fscan_reltargetlist.patch

>         2. Instead of using rel->reltargetlist, we should use the tlist
>         passed
>         by caller. This is the tlist expected from the Plan node. For
>         foreign
>         scans it will be same as rel->reltargetlist. Using tlist would
>         make the
>         function consistent with create_*scan_plan functions.

>     I disagree with that for the reasons mentioned below:
>
>     * For a foreign scan, tlist is *not* necessarily the same as
>     rel->reltargetlist (ie, there is a case where tlist contains all
>     user attributes while rel->reltargetlist contains only attributes
>     actually needed by the query).  In such a case it'd be inefficient
>     to use tlist rather than rel->reltargetlist.

> create_foreignscan_plan() is called from create_scan_plan(), which
> passes the tlist. The later uses function use_physical_tlist() to decide
> which targetlist should be used (physical or actual). As per code below
> in this function
>
>   485     /*
>   486      * We can do this for real relation scans, subquery scans,
> function scans,
>   487      * values scans, and CTE scans (but not for, eg, joins).
>   488      */
>   489     if (rel->rtekind != RTE_RELATION &&
>   490         rel->rtekind != RTE_SUBQUERY &&
>   491         rel->rtekind != RTE_FUNCTION &&
>   492         rel->rtekind != RTE_VALUES &&
>   493         rel->rtekind != RTE_CTE)
>   494         return false;
>   495
>   496     /*
>   497      * Can't do it with inheritance cases either (mainly because
> Append
>   498      * doesn't project).
>   499      */
>   500     if (rel->reloptkind != RELOPT_BASEREL)
>   501         return false;
>
> For foreign tables as well as the tables under inheritance hierarchy it
> uses the actual targetlist, which contains only the required columns IOW
> rel->reltargetlist (see build_path_tlist()) with nested loop parameters
> substituted. So, it never included unnecessary columns in the
> targetlist.

Maybe I'm missing something, but I think you are overlooking the case 
for foreign tables that are *not* under an inheritance hierarchy.  (Note 
that the rtekind for foreign tables is RTE_RELATION.)

Thanks,

Best regards,
Etsuro Fujita