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

On 2016/11/22 15:24, Etsuro Fujita wrote:
> On 2016/11/22 4:49, Tom Lane wrote:
>> Etsuro Fujita <fujita.etsuro@lab.ntt.co.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.4090903@lab.ntt.co.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