Re: postgres_fdw : altering foreign table not invalidating prepare statement execution plan.

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Amit Langote <amitlangote09(at)gmail(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw : altering foreign table not invalidating prepare statement execution plan.
Date: 2016-11-30 08:25:04
Message-ID: 870003ef-5e5a-e223-52b1-6531461d4786@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2016/11/22 15:24, Etsuro Fujita wrote:
> On 2016/11/22 4:49, Tom Lane wrote:
>> Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>>> On 2016/11/10 5:17, Tom Lane wrote:
>>>> I think there's a very good argument that we should just treat any
>>>> inval
>>>> in pg_foreign_data_wrapper as a reason to blow away the whole plan
>>>> cache.
>>>> People aren't going to alter FDW-level options often enough to make it
>>>> worth tracking things more finely. Personally I'd put
>>>> pg_foreign_server
>>>> changes in the same boat, though maybe those are changed slightly more
>>>> often. If it's worth doing anything more than that, it would be to
>>>> note
>>>> which plans mention foreign tables at all, and not invalidate those
>>>> that
>>>> don't. We could do that with, say, a hasForeignTables bool in
>>>> PlannedStmt.

>>> I agree on that point.

>> OK, please update the patch to handle those catalogs that way.

> Will do.

Done. I modified the patch so that any inval in pg_foreign_server also
blows the whole plan cache.

>>>> That leaves the question of whether it's worth detecting table-level
>>>> option changes this way, or whether we should just handle those by
>>>> forcing
>>>> a relcache inval in ATExecGenericOptions, as in Amit's original
>>>> patch in
>>>> <5702298D(dot)4090903(at)lab(dot)ntt(dot)co(dot)jp>. I kind of like that approach; that
>>>> patch was short and sweet, and it put the cost on the unusual path
>>>> (ALTER
>>>> TABLE) not the common path (every time we create a query plan).

>>> I think it'd be better to detect table-level option changes because that
>>> could allow us to do more;

>> Why not? But the bigger picture here is that relcache inval is the
>> established practice for forcing replanning due to table-level changes,
>> and I have very little sympathy for inventing a new mechanism for that
>> just for foreign tables. If it were worth bypassing replanning for
>> changes in tables in dead subqueries, for instance, it would surely be
>> worth making that happen for all table types not just foreign tables.

> My point here is that FDW-option changes wouldn't affect replanning by
> core; even if forcing a replan, we would have the same foreign tables
> excluded by constraint exclusion, for example. So, considering updates
> on pg_foreign_table to be rather frequent, it might be better to avoid
> replanning for the pg_foreign_table changes to foreign tables excluded
> by constraint exclusion, for example. My concern about the
> relcache-inval approach is: that approach wouldn't allow us to do that,
> IIUC.

I thought updates on pg_foreign_table would be rather frequent, but we
don't prove that that is enough that more-detailed tracking is worth the
effort. Yes, as you mentioned, it wouldn't be a good idea to invent the
new mechanism just for foreign tables. So, +1 for relcache inval.
Attached is an updated version of the patch, in which I also modified
regression tests; re-created tests to check to see if execution as well
as explain work properly and added some tests for altering server-level
options.

Sorry for the delay.

Best regards,
Etsuro Fujita

Attachment Content-Type Size
foreign-plan-inval.patch text/x-patch 12.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2016-11-30 08:29:47 Re: Push down more UPDATEs/DELETEs in postgres_fdw
Previous Message Amit Langote 2016-11-30 08:01:02 Minor correction in alter_table.sgml