Re: Problem while updating a foreign table pointing to a partitioned table on foreign server

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp
Cc: ashutosh(dot)bapat(at)enterprisedb(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, alvherre(at)2ndquadrant(dot)com, robertmhaas(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Problem while updating a foreign table pointing to a partitioned table on foreign server
Date: 2018-08-24 07:58:27
Message-ID: 20180824.165827.226719997.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello.

At Tue, 21 Aug 2018 11:01:32 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20180821(dot)110132(dot)261184472(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> > You wrote:
> > > Several places seems to be assuming that fdw_scan_tlist may be
> > > used foreign scan on simple relation but I didn't find that
> > > actually happens.
> >
> > Yeah, currently, postgres_fdw and file_fdw don't use that list for
> > simple foreign table scans, but it could be used to improve the
> > efficiency for those scans, as explained in fdwhandler.sgml:
...
> I'll put more consideration on using fdw_scan_tlist in the
> documented way.

Done. postgres_fdw now generates full fdw_scan_tlist (as
documented) for foreign relations with junk columns having a
small change in core side. However it is far less invasive than
the previous version and I believe that it dones't harm
maybe-existing use of fdw_scan_tlist on non-join rels (that is,
in the case of a subset of relation columns).

The previous patch didn't show "tableoid" in the Output list (as
"<added_junk>") of explain output but this does correctly by
referring to rte->eref->colnames. I believe no other FDW has
expanded foreign relation even if it uses fdw_scan_tlist for
ForeignScan on a base relation so it won't harm them.

One arguable behavior change is about wholrow vars. Currently it
refferes local tuple with all columns but it is explicitly
fetched as ROW() after this patch applied. This could be fixed
but not just now.

Part of 0004-:
- Output: f1, ''::text, ctid, rem1.*
- Remote SQL: SELECT f1, f2, ctid FROM public.loc1 FOR UPDATE
+ Output: f1, ''::text, tableoid, ctid, rem1.*
+ Remote SQL: SELECT f1, tableoid, ctid, ROW(f1, f2) FROM public.loc1 FOR UPDATE

Since this uses fdw_scan_tlist so it is theoretically
back-patchable back to 9.6. This patch applies on top of the
current master.

Please find the attached three files.

0001-Add-test-for-postgres_fdw-foreign-parition-update.patch

This should fail for unpatched postgres_fdw. (Just for demonstration)

0002-Core-side-modification-for-PgFDW-foreign-update-fix.patch

Core side change which allows fdw_scan_tlist to have extra
columns that is not defined in the base relation.

0003-Fix-of-foreign-update-bug-of-PgFDW.patch

Fix of postgres_fdw for this problem.

0004-Regtest-change-for-PgFDW-foreign-update-fix.patch

Regression test change separated for readability.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-Add-test-for-postgres_fdw-foreign-parition-update.patch text/x-patch 5.1 KB
0002-Core-side-modification-for-PgFDW-foreign-update-fix.patch text/x-patch 6.2 KB
0003-Fix-of-foreign-update-bug-of-PgFDW.patch text/x-patch 23.0 KB
0004-Regtest-change-for-PgFDW-foreign-update-fix.patch text/x-patch 23.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2018-08-24 08:06:07 Re: Problem while updating a foreign table pointing to a partitioned table on foreign server
Previous Message Tatsuro Yamada 2018-08-24 05:04:15 Re: [HACKERS] CLUSTER command progress monitor