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