(2018/08/08 17:30), Kyotaro HORIGUCHI wrote:
> Please find the attached.
Thanks for the patch, Horiguchi-san!
> At Fri, 3 Aug 2018 11:48:38 +0530, Ashutosh Bapat<ashutosh.bapat@enterprisedb.com> wrote
in<CAFjFpRcF-j+B8W8o-wrvOguA0=r8SJ-rCrzWAnHT2V66NxGfFQ@mail.gmail.com>
>> The purpose of non-Var node is to avoid adding the attribute to
>> relation descriptor. Idea is to create a new node, which will act as a
>> place holder for table oid or row id (whatever) to be fetched from the
>> foreign server.
I think so too.
>> I don't understand why do you think we need it to be
>> added to the relation descriptor.
I don't understand that either.
> I choosed to expand tuple descriptor for junk column added to
> foreign relaions. We might be better to have new member in
> ForeignScan but I didn't so that we can backpatch it.
I've not looked at the patch closely yet, but I'm not sure that it's a
good idea to expand the tuple descriptor of the target relation on the
fly so that it contains the remotetableoid as a non-system attribute of
the target table. My concern is: is there not any risk in affecting
some other part of the planner and/or the executor? (I was a bit
surprised that the patch passes the regression tests successfully.)
To avoid expanding the tuple descriptor, I'm wondering whether we could
add a Param representing remotetableoid, not a Var undefined anywhere in
the system catalogs, as mentioned above?
> What the patch does are:
>
> - This abuses ForeignScan->fdw_scan_tlist to store the additional
> junk columns when foreign simple relation scan (that is, not a
> join).
I think that this issue was introduced in 9.3, which added postgres_fdw
in combination with support for writable foreign tables, but
fdw_scan_tlist was added to 9.5 as part of join-pushdown infrastructure,
so my concern is that we might not be able to backpatch your patch to
9.3 and 9.4.
That's it for now.
Best regards,
Etsuro Fujita
