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

Hi,

At Tue, 5 Apr 2016 19:46:04 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote in <5703976C(dot)30308(at)lab(dot)ntt(dot)co(dot)jp>
> On 2016/04/05 18:44, Kyotaro HORIGUCHI wrote:
> > At Tue, 5 Apr 2016 14:24:52 +0900, Amit Langote wrote:
> > With this patch, making any change on foreign servers or user
> > mappings corresponding to exiting connection causes
> > disconnection. This could be moved in to core, with the following
> > API like this.
> >
> > reoutine->NotifyObjectModification(ObjectAccessType access,
> > Oid classId, Oid objId);
> >
> > - using object access hook (which is put by the core itself) is not bad?
> >
> > - If ok, the API above looks bad. Any better API?
>
> Although helps achieve our goal here, object access hook stuff seems to be
> targeted at different users:
>
> /*
> * Hook on object accesses. This is intended as infrastructure for security
> * and logging plugins.
> */
> object_access_hook_type object_access_hook = NULL;

Yeah, furthermore, it doesn't notify to other backends, that is
what I forgotten at the time:(

So, it seems reasonable that all stuffs ride on the cache
invalidation mechanism.

Object class id and object id should be necessary to be
adequitely notificated to fdws but the current invalidation
mechanism donesn't convey the latter. It is hidden in hash
code. This is resolved just by additional member in PlanInvalItem
or resolving oid from hash code(this would need catalog scan..).

PlanCache*Callback() just do invalidtion and throw the causeal
PlanInvalItem away. If plancache had oid lists of foreign
servers, foreign tables, user mappings or FDWS, we can notify
FDWs of invalidation with the causal object.

- Adding CachedPlanSource with some additional oid lists, which
will be built by extract_query_dependencies.

- In PlanCache*Callback(), class id and object id of the causal
object is notified to FDW.

> I just noticed the following comment above GetConnection():
>
> *
> * XXX Note that caching connections theoretically requires a mechanism to
> * detect change of FDW objects to invalidate already established connections.
> * We could manage that by watching for invalidation events on the relevant
> * syscaches. For the moment, though, it's not clear that this would really
> * be useful and not mere pedantry. We could not flush any active connections
> * mid-transaction anyway.
>
>
> So, the hazard of flushing the connection mid-transaction sounds like it
> may be a show-stopper here, even if we research some approach based on
> syscache invalidation. Although I see that your patch takes care of it.

Yeah. Altough cache invalidation would accur amid transaction..

> > By the way, AlterUserMapping seems missing calling
> > InvokeObjectPostAlterHook. Is this a bug?
>
> Probably, yes.

But we dont' see a proper way to "fix" this since we here don't
use this.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center