Re: postgres_fdw behaves oddly

Hi Fujita-san,

Here are my comments about the patch fscan_reltargetlist.patch

Sanity
--------

Patch applies and compiles cleanly.

Regressions in test/regress folder and postgres_fdw and file_fdw are clean.

1. This isn't your change, but we might be able to get rid of assignment
2071     /* Are any system columns requested from rel? */
2072     scan_plan->fsSystemCol = false;

since make_foreignscan() already does that. But I will leave upto you to remove this assignment or not.

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.

Otherwise, the patch looks good.

On Wed, Nov 12, 2014 at 12:55 PM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:

(2014/11/11 18:45), Etsuro Fujita wrote:

(2014/11/10 20:05), Ashutosh Bapat wrote:

Since two separate issues 1. using reltargetlist instead of attr_needed
and 2. system columns usage in FDW are being tackled here, we should
separate the patch into two one for each of the issues.

Agreed.  Will split the patch into two.

Here are splitted patches:

fscan-reltargetlist.patch  - patch for #1
postgres_fdw-syscol.patch  - patch for #2

Thanks,

Best regards,
Etsuro Fujita

--

Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company