(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.ba...@enterprisedb.com>
wrote in<CAFjFpRcF-j+B8W8o-wrvOguA0=r8sj-rcrzwanht2v66nxg...@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
