Re: postgres_fdw behaves oddly - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: postgres_fdw behaves oddly
Date
Msg-id CAFjFpRfc_byPR-PfU4yKnQA_67u24wxapfs24ywrRhtWHpvGwQ@mail.gmail.com
Whole thread Raw
In response to Re: postgres_fdw behaves oddly  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
Responses Re: postgres_fdw behaves oddly  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
List pgsql-hackers


On Wed, Nov 19, 2014 at 11:18 AM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
(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.)



Ah! you are right. I confused between rtekind and relkind. Sorry for that. May be we should modify use_physical_tlist() to return false in case of RELKIND_FOREIGN_TABLE, so that we can use tlist in create_foreignscan_plan(). I do not see any create_*_plan() function using reltargetlist directly.
 
Thanks,

Best regards,
Etsuro Fujita



--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

pgsql-hackers by date:

Previous
From: Etsuro Fujita
Date:
Subject: Re: postgres_fdw behaves oddly
Next
From: Will Leinweber
Date:
Subject: Re: psql \watch always ignores \pset null