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

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Odd system-column handling in postgres_fdw join pushdown patch
Date: 2016-04-14 11:49:08
Message-ID: CAFjFpRcQHqd0CbtFnn0_ZwdejuY73zpQM7-PT4ruiwsxA0UZww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks Robert for the patch.

> OK, here's a patch. What I did is:
>
> 1. For a regular FDW scan, zero the xmin, xmax, and cid of the tuple
> before returning it from postgres_fdw, so that we don't expose the
> datum-tuple fields. I can't see any reason this isn't safe, but I
> might be missing something.
>
> 2. When a join is pushed down, deparse system columns using something
> like "CASE WHEN r1.* IS NOT NULL THEN 0 END", except for the table OID
> column, which gets deparsed with the table OID in place of 0. This
> delivers the correct behavior in the presence of outer joins.
>
> Review appreciated.
>

It looks good to me. It might be good to explain why we don't add CASE ..
END statement when qualify_col is false, i.e. "qualify_col being false
indicates that there is only one relation involved thus no join."

BTW, I noticed that we are deparsing whole-row reference as ROW(list of
columns from local definition of foreign table), which has the same problem
with outer joins. It won't be NULL when the rest of the row from that
relation is NULL in an outer join. It too needs to be encapsulated in CASE
WHEN .. END expression. PFA patch with that fix included and also some
testcases for system columns as well as whole-row references.

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

Attachment Content-Type Size
postgres-fdw-syscol-zap-ab.patch text/x-diff 45.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-04-14 12:04:29 Re: Support for N synchronous standby servers - take 2
Previous Message Michael Paquier 2016-04-14 11:41:41 Re: [COMMITTERS] pgsql: Add regression tests for multiple synchronous standbys.