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

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Langote <amitlangote09(at)gmail(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw : altering foreign table not invalidating prepare statement execution plan.
Date: 2016-10-04 11:30:18
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2016/09/30 1:09, Ashutosh Bapat wrote:

You wrote:
>>> The attached patch adds the
>>> dependencies from create_foreignscan_plan() which is called for any
>>> foreign access. I have also added testcases to test the functionality.
>>> Let me know your comments on the patch.

I wrote:
>> Hmm. I'm not sure that that's a good idea.
>> I was thinking the changes to setrefs.c proposed by Amit to collect that
>> dependencies would be probably OK, but I wasn't sure that it's a good idea
>> that he used PlanCacheFuncCallback as the syscache inval callback function
>> for the foreign object caches because it invalidates not only generic plans
>> but query trees, as you mentioned downthread. So, I was thinking to modify
>> his patch so that we add a new syscache inval callback function for the
>> caches that is much like PlanCacheFuncCallback but only invalidates generic
>> plans.

> PlanCacheFuncCallback() invalidates the query tree only when
> invalItems are added to the plan source. The patch adds the
> dependencies in root->glob->invalItems, which standard_planner()
> copies into PlannedStmt::invalItems. This is then copied into the
> gplan->stmt_list. Thus PlanCacheFuncCallback never invalidates the
> query tree. I have verified this under the debugger. Am I missing
> something?

I think you are right. Maybe my explanation was not enough, sorry for
that. I just wanted to mention that Amit's patch would invalidate the
generic plan as well as the rewritten query tree.

I looked at your patch closely. You added code to track dependencies on
FDW-related objects to createplan.c, but I think it would be more
appropriate to put that code in setrefs.c like the attached. I think
record_foreignscan_plan_dependencies in your patch would be a bit
inefficient because that tracks such dependencies redundantly in the
case where the given ForeignScan has an outer subplan, so I optimized
that in the attached. (Also some regression tests for that were added.)

I think it would be a bit inefficient to use PlanCacheFuncCallback as
the inval callback function for those caches, because that checks the
inval-item list for the rewritten query tree, but any updates on such
catalogs wouldn't affect the query tree. So, I added a new callback
function for those caches that is much like PlanCacheFuncCallback but
skips checking the list for the query tree. I updated some comments also.

Best regards,
Etsuro Fujita

Attachment Content-Type Size
foreign_plan_cache_inval_v2.patch binary/octet-stream 23.0 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Julian Markwort 2016-10-04 11:42:26 Re: [PATCH] pgpassfile connection option
Previous Message Ashutosh Bapat 2016-10-04 11:29:48 Re: Transactions involving multiple postgres foreign servers