Re: Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Mar 23, 2016 at 8:20 AM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
wrote:

> On 2016/03/22 21:10, Ashutosh Bapat wrote:
>
>> On Tue, Mar 22, 2016 at 5:05 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:
>> On 2016/03/22 14:54, Ashutosh Bapat wrote:
>> Earlier in this mail chain, I suggested that the core should
>> take care
>> of storing the values for these columns. But instead, I think,
>> core
>> should provide functions which can be used by FDWs, if they want,
>> to
>> return values for those columns. Something like Datum
>> get_syscol_value(RelOptInfo/Relation, attno). The function will
>> return
>> Datum 0 for most of the columns and table's OID for tableoid. My
>> 0.02.
>>
>
> What I had in mind was (1) create_foreignscan_plan would create
>> Lists from the ForeignScan's fdw_scan_tlist: (a) indexes/OID values
>> of tableoids in fdw_scan_tlist, and (b) indexes of xids and cids in
>> fdw_scan_tlist, and then (2) ForeignNext would set the OID values
>> for the tableoid columns in the scan tuple, using the Lists (a), and
>> appropriate values (0 or something) for the xid and cid columns in
>> the scan tuple, using the List (b).
>>
>
> Looks Ok to me, although, that way an FDW looses an ability to use its
>> own values for those columns, in case it wants to. For example, while
>> using postgres_fdw for sharding, it might help saving xmax, xmin, cmax,
>> cmin from the foreign server and use them while communicating with the
>> foreign server.
>>
>
> Yeah, it might be the case.
>
> On second thoughts, I changed my mind; I think it'd be better for the
> FDW's to set values for tableoids, xids, and cids in the scan tuple. The
> reason other than your suggestion is because expressions in fdw_scan_tlist
> that contain such columns are not necessarily simple Vars and because such
> expressions might be evaluated more efficiently by the FDW than core. We
> assume in postgres_fdw that expressions in fdw_scan_tlist are always simple
> Vars, though.
>
> I'm not sure it's worth providing functions you suggested, because we
> can't assume that columns in the scan tuple are always simple Var columns,
> as I said above.
>
>
An FDW can choose not to use those functions, so I don't see a connection
between scan list having simple Vars and existence of those functions
(actually a single one). But having those function would minimize the code
that each FDW has to write, in case they want those functions. E.g. we have
to translate Var::varno to tableoid in case that's requested by pulling RTE
and then getting oid out from there. If that functionality is available in
the core, 1. the code is not duplicated 2. every FDW will get the same
tableoid. Similarly for the other columns.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company