Re: postgres_fdw behaves oddly

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw behaves oddly
Date: 2014-11-19 05:48:36
Message-ID: 546C2F34.1000901@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2014/11/18 18:27), Ashutosh Bapat wrote:
> On Tue, Nov 18, 2014 at 1:50 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2014-11-19 05:53:53 Re: REINDEX CONCURRENTLY 2.0
Previous Message Tom Lane 2014-11-19 05:48:34 Re: Multiple call of GetTransactionSnapshot in single flow