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

At Tue, 5 Apr 2016 14:24:52 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote in <57034C24(dot)1000203(at)lab(dot)ntt(dot)co(dot)jp>
> On 2016/04/05 0:23, Tom Lane wrote:
> > Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> >> On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>> A related issue, now that I've seen this example, is that altering
> >>> FDW-level or server-level options won't cause a replan either. I'm
> >>> not sure there's any very good fix for that. Surely we don't want
> >>> to try to identify all tables belonging to the FDW or server and
> >>> issue relcache invals on all of them.
> >
> >> Hm, some kind of PlanInvalItem-based solution could work maybe?
> >
> > Hm, so we'd expect that whenever an FDW consulted the options while
> > making a plan, it'd have to record a plan dependency on them? That
> > would be a clean fix maybe, but I'm worried that third-party FDWs
> > would fail to get the word about needing to do this.
>
> I would imagine that that level of granularity may be a little too much; I
> mean tracking dependencies at the level of individual FDW/foreign
> table/foreign server options. I think it should really be possible to do
> the entire thing in core instead of requiring this to be made a concern of
> FDW authors. How about the attached that teaches
> extract_query_dependencies() to add a foreign table and associated foreign
> data wrapper and foreign server to invalItems. Also, it adds plan cache
> callbacks for respective caches.

It seems to work but some renaming of functions would needed.

> One thing that I observed that may not be all that surprising is that we
> may need a similar mechanism for postgres_fdw's connection cache, which
> doesn't drop connections using older server connection info after I alter
> them. I was trying to test my patch by altering dbaname option of a
> foreign server but that was silly, ;). Although, I did confirm that the
> patch works by altering use_remote_estimates server option. I could not
> really test for FDW options though.
>
> Thoughts?

It seems a issue of FDW drivers but notification can be issued by
the core. The attached ultra-POC patch does it.

Disconnecting of a fdw connection with active transaction causes
an unexpected rollback on the remote side. So disconnection
should be allowed only when xact_depth == 0 in
pgfdw_subxact_callback().

There are so many parameters that affect connections, which are
listed as PQconninfoOptions. It is a bit too complex to detect
changes of one or some of them. So, I try to use object access
hook even though using hook is not proper as fdw interface. An
additional interface would be needed to do this anyway.

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?

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

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center