On 2016/10/06 20:17, Amit Langote wrote:
On 2016/10/05 20:45, Etsuro Fujita wrote:
On 2016/10/05 14:09, Ashutosh Bapat wrote:

I wrote:
So, I added a new callback function for those caches
that is much like PlanCacheFuncCallback but skips checking the list for
query tree.

IMO, maintaining that extra function and
the risk of bugs because of not keeping those two functions in sync
outweigh the small not-so-frequent gain.

The inefficiency wouldn't be negligible in the case where there are large
numbers of cached plans.  So, I'd like to introduce a helper function that
checks the dependency list for the generic plan, to eliminate most of the

I too am inclined to have a PlanCacheForeignCallback().

Just one minor comment:

Thanks for the comments!

I noticed that we were wrong. Your patch was modified so that dependencies on FDW-related objects would be extracted from a given plan in create_foreignscan_plan (by Ashutosh) and then in set_foreignscan_references by me, but that wouldn't work well for INSERT cases. To fix that, I'd like to propose that we collect the dependencies from the given rte in add_rte_to_flat_rtable, instead.

Attached is an updated version, in which I removed the PlanCacheForeignCallback and adjusted regression tests a bit.

If some writable FDW consulted foreign table/server/FDW options in
AddForeignUpdateTarget, which adds the extra junk columns for
UPDATE/DELETE to the targetList of the given query tree, the rewritten
query tree would also need to be invalidated.  But I don't think such an
FDW really exists because that routine in a practical FDW wouldn't change
such columns depending on those options.

I had second thoughts about that; since the possibility wouldn't be zero, I added to extract_query_dependencies_walker the same code I added to add_rte_to_flat_rtable.

After all, the updated patch is much like your version, but I think your patch, which modified extract_query_dependencies_walker only, is not enough because a generic plan can have more dependencies than its query tree (eg, consider inheritance).

Best regards,
Etsuro Fujita

Attachment: foreign_plan_cache_inval_v4.patch
Description: binary/octet-stream

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to