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

On 2016/11/10 5:17, Tom Lane wrote:
> Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> writes:
>> [ foreign_plan_cache_inval_v6.patch ]

> I looked through this a bit.

Thank you for working on this!

> A point that doesn't seem to have been
> discussed anywhere in the thread is why foreign tables should deserve
> exact tracking of dependencies, when we have not bothered with that
> for most other object types.  Schemas, for example, we're happy to just
> zap the whole plan cache for.  Attempting to do exact tracking is
> somewhat expensive and bug-prone --- the length of this thread speaks
> to the development cost and bug hazard.  Meanwhile, nobody has questioned
> the cost of attaching more PlanInvalItems to a plan (and the cost of the
> extra catalog lookups this patch does to create them).  For most plans,
> that's nothing but overhead because no invalidation will actually occur
> in the life of the plan.
>
> 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.

> 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; we could avoid invalidating cached plans that 
need not to be invalidated, by tracking the plan dependencies more 
exactly, ie, avoid collecting the dependencies of foreign tables in a 
plan that are in dead subqueries or excluded by constraint exclusion. 
The proposed patch doesn't do that, though.  I think updates on 
pg_foreign_table would be more frequent, so it might be worth 
complicating the code.  But the relcache-inval approach couldn't do 
that, IIUC.

Best regards,
Etsuro Fujita