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

On 2016/10/17 20:12, Ashutosh Bapat wrote:

>> On 2016/10/07 10:26, Amit Langote wrote:
>>> I think this (v4) patch is in the best shape so far.
> +1, except one small change.

> The comment "... On relcache invalidation events or the relevant
> syscache invalidation events, ..." specifies relcache separately. I
> think, it should either needs to specify all the syscaches or just
> mention "On corresponding syscache invalidation events, ...".
>
> Attached patch uses the later version. Let me know if this looks good.
> If you are fine, I think we should mark this as "Ready for committer".

I'm not sure that is a good idea because ISTM the comments there use the 
words "syscache" and "relcache" properly.  I'd like to leave that for 
committers.

> The patch compiles cleanly, and make check in regress and that in
> postgres_fdw shows no failures.

Thanks for the review and the updated patch!

One thing I'd like to propose to improve the patch is to update the 
following comments for PlanCacheFuncCallback: "Note that the coding 
would support use for multiple caches, but right now only user-defined 
functions are tracked this way.".  We now use this for tracking 
FDW-related objects as well, so the comments needs to be updated. 
Please find attached an updated version.

Sorry for speaking this now, but one thing I'm not sure about the patch 
is whether we should track the dependencies on FDW-related objects more 
accurately; we modified extract_query_dependencies so that the query 
tree's dependencies are tracked, regardless of the command type, but the 
query tree would be only affected by those objects in 
AddForeignUpdateTargets, so it would be enough to collect the 
dependencies for the case where the given query is UPDATE/DELETE.  But I 
thought the patch would be probably fine as proposed, because we expect 
updates on such catalogs to be infrequent.  (I guess the changes needed 
for the accuracy would be small, though.)  Besides that, I modified 
add_rte_to_flat_rtable so that the plan's dependencies are tracked, but 
that would lead to tracking the dependencies of unreferenced foreign 
tables in dead subqueries or the dependencies of foreign tables excluded 
from the plan by eg, constraint exclusion.  But I thought that would be 
also OK by the same reason as above.  (Another reason for that was it 
seemed better to me to collect the dependencies in the same place as for 
relation OIDs.)

Best regards,
Etsuro Fujita