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(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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