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

A much simpler solution, that will work with postgres_fdw, might be to just
deparse these columns with whatever random values (except for tableoid)
they are expected to have in those places. Often these values can simply be
NULL or 0. For tableoid deparse it to 'oid value'::oid. Thus for a user
query

select t1.taleoid, t2.xmax, t1.c1, t2.c2 from t1 join t2 on (...) ... --
where t1 and t2 are foreign tables with same names on the foreign server.

the query sent to the foreign server would look like

select '15623'::oid, NULL, t1.c1, t2.c2 from t1 join t2 on (...) ... --
where '15623' is oid of t1 on local server.

This does spend more bandwidth than necessary and affect performance, here
is why the approach might be better,
1. It's not very common to request these system columns in a "join" query
involving foreign tables. Usually they will have user columns or ctid
(DMLs) but very rarely other system columns.

2. This allows expressions involving these system columns to be pushed
down, whenever we will start pushing them down in the targetlist.

3. The changes to the code are rather small. deparseColumnRef() will need
to produce the strings above instead of actual column names.

4. The approach will work with slight change, if and when, we need the
actual system column values from the foreign server. That time the above
function needs to deparse the column names instead of constant values.

Having to hardcode tableoid at the time of planning should be fine since
change in tableoid between planning and execution will trigger plan cache
invalidation. I haven't tried this though.

Sorry for bringing this solution late to the table.

On Thu, Mar 24, 2016 at 3:04 PM, Ashutosh Bapat <
ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:

>
>
> On Thu, Mar 24, 2016 at 9:31 AM, Etsuro Fujita <
> fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>
>> On 2016/03/23 13:44, Ashutosh Bapat wrote:
>>
>>> 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.
>>>
>>
>> OK. Then, I'd like to propose a function that would create interger
>> Lists of indexes of tableoids, xids and cids plus
>
>
> Ok,
>
>
>> an OID List of these tableoids,
>
>
> I didn't get this.
>
>
>> in a given fdw_scan_tlist, on the assumption that each expression in the
>> fdw_scan_tlist is a simple Var.
>
>
> I guess this is Ok. In fact, at least for now an expression involving any
> of those columns is not pushable to the foreign server, as the expression
> can not be evaluated there. So, if we come across such a case in further
> pushdowns, we will need to have a different solution for pushing down such
> target lists.
>
>
>> I'd also like to propose another function that would fill system columns
>> using these Lists when creating a scan tuple.
>>
>> Ok.
>
> I had imagined that the code to extract the above lists and filling the
> values in scan tuple will be in FDW. We only provide a function to supply
> those values. But what you propose might actually be much practical.
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
>

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