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

2017-01-06 Thread Tom Lane
Etsuro Fujita writes: > On 2017/01/06 21:25, Ashutosh Bapat wrote: >> On Thu, Jan 5, 2017 at 2:49 PM, Etsuro Fujita >> wrote: >>> On 2017/01/03 15:57, Ashutosh Bapat wrote: The patch looks good to me, but I feel there are too many

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

2017-01-06 Thread Etsuro Fujita
On 2017/01/06 21:25, Ashutosh Bapat wrote: On Thu, Jan 5, 2017 at 2:49 PM, Etsuro Fujita wrote: On 2017/01/03 15:57, Ashutosh Bapat wrote: The patch looks good to me, but I feel there are too many testscases. Now that we have changed the approach to invalidate

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

2017-01-06 Thread Ashutosh Bapat
On Thu, Jan 5, 2017 at 2:49 PM, Etsuro Fujita wrote: > On 2017/01/03 15:57, Ashutosh Bapat wrote: >> >> The patch looks good to me, but I feel there are too many testscases. >> Now that we have changed the approach to invalidate caches in all >> cases, should we just

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

2017-01-05 Thread Etsuro Fujita
On 2017/01/03 15:57, Ashutosh Bapat wrote: The patch looks good to me, but I feel there are too many testscases. Now that we have changed the approach to invalidate caches in all cases, should we just include cases for SELECT or UPDATE or INSERT or DELETE instead of each statement? I don't

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

2017-01-02 Thread Ashutosh Bapat
On Wed, Nov 30, 2016 at 2:35 PM, Etsuro Fujita wrote: > On 2016/11/30 17:53, Amit Langote wrote: >> >> On 2016/11/30 17:25, Etsuro Fujita wrote: >>> >>> Done. I modified the patch so that any inval in pg_foreign_server also >>> blows the whole plan cache. > > >> I

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

2016-12-04 Thread Haribabu Kommi
On Wed, Nov 30, 2016 at 8:05 PM, Etsuro Fujita wrote: > On 2016/11/30 17:53, Amit Langote wrote: > >> On 2016/11/30 17:25, Etsuro Fujita wrote: >> >>> Done. I modified the patch so that any inval in pg_foreign_server also >>> blows the whole plan cache. >>> >> > I

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

2016-11-30 Thread Etsuro Fujita
On 2016/11/30 17:53, Amit Langote wrote: On 2016/11/30 17:25, Etsuro Fujita wrote: Done. I modified the patch so that any inval in pg_foreign_server also blows the whole plan cache. I noticed the following addition: + CacheRegisterSyscacheCallback(FOREIGNDATAWRAPPEROID,

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

2016-11-30 Thread Amit Langote
Fujita-san, On 2016/11/30 17:25, Etsuro Fujita wrote: > On 2016/11/22 15:24, Etsuro Fujita wrote: >> On 2016/11/22 4:49, Tom Lane wrote: > >>> OK, please update the patch to handle those catalogs that way. > >> Will do. > > Done. I modified the patch so that any inval in pg_foreign_server

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

2016-11-30 Thread Etsuro Fujita
On 2016/11/22 15:24, Etsuro Fujita wrote: On 2016/11/22 4:49, Tom Lane wrote: Etsuro Fujita writes: On 2016/11/10 5:17, Tom Lane wrote: I think there's a very good argument that we should just treat any inval in pg_foreign_data_wrapper as a reason to blow away

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

2016-11-21 Thread Etsuro Fujita
On 2016/11/22 4:49, Tom Lane wrote: Etsuro Fujita writes: On 2016/11/10 5:17, Tom Lane wrote: I think there's a very good argument that we should just treat any inval in pg_foreign_data_wrapper as a reason to blow away the whole plan cache. People aren't going to

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

2016-11-21 Thread Tom Lane
[ apologies for not responding sooner ] Etsuro Fujita writes: > On 2016/11/10 5:17, Tom Lane wrote: >> I think there's a very good argument that we should just treat any inval >> in pg_foreign_data_wrapper as a reason to blow away the whole plan cache. >> People

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

2016-11-10 Thread Ashutosh Bapat
> > That leaves the question of whether it's worth detecting table-level > option changes this way, or whether we should just handle those by forcing > a relcache inval in ATExecGenericOptions, as in Amit's original patch in > <5702298d.4090...@lab.ntt.co.jp>. I kind of like that approach; that >

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

2016-11-09 Thread Etsuro Fujita
On 2016/11/10 5:17, Tom Lane wrote: Etsuro Fujita writes: [ foreign_plan_cache_inval_v6.patch ] I looked through this a bit. Thank you for working on this! A point that doesn't seem to have been discussed anywhere in the thread is why foreign tables should

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

2016-11-09 Thread Tom Lane
Etsuro Fujita writes: > [ foreign_plan_cache_inval_v6.patch ] I looked through this a bit. A point that doesn't seem to have been discussed anywhere in the thread is why foreign tables should deserve exact tracking of dependencies, when we have not bothered with

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

2016-10-20 Thread Etsuro Fujita
On 2016/10/20 16:27, Ashutosh Bapat wrote: I wrote: 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

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

2016-10-20 Thread Ashutosh Bapat
> > >> In fact, I am not in >> favour of tracking the query dependencies for UPDATE/DELETE since we >> don't have any concrete example as to when that would be needed. > > > Right, but as I said before, some FDW might consult FDW options stored in > those objects during AddForeignUpdateTargets, so

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

2016-10-18 Thread Amit Langote
On 2016/10/19 12:20, Etsuro Fujita wrote: > Having said that, I like the latest version (v6), so I'd vote for marking > this as Ready For Committer. +1, thanks a lot! Regards, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

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

2016-10-18 Thread Etsuro Fujita
On 2016/10/18 22:04, Ashutosh Bapat wrote: On Tue, Oct 18, 2016 at 6:18 PM, Etsuro Fujita wrote: 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

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

2016-10-18 Thread Ashutosh Bapat
On Tue, Oct 18, 2016 at 6:18 PM, Etsuro Fujita wrote: > 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 "...

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

2016-10-18 Thread Etsuro Fujita
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

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

2016-10-17 Thread Ashutosh Bapat
On Fri, Oct 7, 2016 at 7:20 AM, Etsuro Fujita wrote: > On 2016/10/07 10:26, Amit Langote wrote: >> >> On 2016/10/06 21:55, Etsuro Fujita wrote: >>> >>> On 2016/10/06 20:17, Amit Langote wrote: On 2016/10/05 20:45, Etsuro Fujita wrote: > > >>> I noticed that

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

2016-10-06 Thread Etsuro Fujita
On 2016/10/07 10:26, Amit Langote wrote: On 2016/10/06 21:55, Etsuro Fujita wrote: On 2016/10/06 20:17, Amit Langote wrote: On 2016/10/05 20:45, Etsuro Fujita wrote: I noticed that we were wrong. Your patch was modified so that dependencies on FDW-related objects would be extracted from a

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

2016-10-06 Thread Amit Langote
On 2016/10/06 21:55, Etsuro Fujita wrote: > 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: IMO, maintaining that extra function and the risk of bugs because of not keeping those two functions in sync

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

2016-10-06 Thread Etsuro Fujita
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 the query tree. IMO,

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

2016-10-06 Thread Amit Langote
Thanks to both of you for taking this up and sorry about the delay in responding. On 2016/10/05 20:45, Etsuro Fujita wrote: > On 2016/10/05 14:09, Ashutosh Bapat wrote: >>> I think it would be a bit inefficient to use PlanCacheFuncCallback as the >>> inval callback function for those caches,

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

2016-10-05 Thread Etsuro Fujita
On 2016/10/05 14:09, Ashutosh Bapat wrote: I wrote: 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. +

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

2016-10-04 Thread Ashutosh Bapat
> > 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

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

2016-10-04 Thread Etsuro Fujita
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

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

2016-10-02 Thread Michael Paquier
On Fri, Sep 30, 2016 at 1:09 AM, Ashutosh Bapat 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

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

2016-09-29 Thread Ashutosh Bapat
> >> 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. > > > Hmm. I'm not sure that that's a good idea. > > I was thinking the

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

2016-09-29 Thread Etsuro Fujita
On 2016/09/29 20:03, Ashutosh Bapat wrote: On Tue, Apr 5, 2016 at 10:54 AM, Amit Langote wrote: 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

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

2016-09-29 Thread Etsuro Fujita
On 2016/09/29 20:06, Ashutosh Bapat wrote: On Wed, Aug 24, 2016 at 1:55 PM, Etsuro Fujita wrote: On 2016/04/04 23:24, Tom Lane 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.

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

2016-09-29 Thread Ashutosh Bapat
On Wed, Aug 24, 2016 at 1:55 PM, Etsuro Fujita wrote: > On 2016/04/04 23:24, Tom Lane wrote: >> >> Amit Langote writes: >>> >>> On 2016/04/04 15:17, Rajkumar Raghuwanshi wrote: * .Observation: *Prepare statement execution plan

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

2016-09-29 Thread Ashutosh Bapat
On Tue, Apr 5, 2016 at 10:54 AM, Amit Langote wrote: > On 2016/04/05 0:23, Tom Lane wrote: >> Amit Langote writes: >>> On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane wrote: A related issue, now that I've seen this

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

2016-08-24 Thread Etsuro Fujita
On 2016/04/04 23:24, Tom Lane wrote: Amit Langote writes: On 2016/04/04 15:17, Rajkumar Raghuwanshi wrote: * .Observation: *Prepare statement execution plan is not getting changed even after altering foreign table to point to new schema. I wonder if

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

2016-04-14 Thread Amit Langote
On 2016/04/05 14:24, Amit Langote wrote: > On 2016/04/05 0:23, Tom Lane wrote: >> Amit Langote writes: >>> 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

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

2016-04-06 Thread Kyotaro HORIGUCHI
Hi, At Tue, 5 Apr 2016 19:46:04 +0900, Amit Langote wrote in <5703976c.30...@lab.ntt.co.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

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

2016-04-05 Thread Amit Langote
On 2016/04/05 18:44, Kyotaro HORIGUCHI wrote: > At Tue, 5 Apr 2016 14:24:52 +0900, Amit Langote wrote: >> On 2016/04/05 0:23, Tom Lane wrote: >>> Amit Langote writes: Hm, some kind of PlanInvalItem-based solution could work maybe? >>> >>> Hm, so we'd expect that

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

2016-04-05 Thread Kyotaro HORIGUCHI
At Tue, 5 Apr 2016 14:24:52 +0900, Amit Langote wrote in <57034c24.1000...@lab.ntt.co.jp> > On 2016/04/05 0:23, Tom Lane wrote: > > Amit Langote writes: > >> On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane wrote: > >>> A

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

2016-04-04 Thread Amit Langote
On 2016/04/05 0:23, Tom Lane wrote: > Amit Langote writes: >> On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane 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.

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

2016-04-04 Thread Kyotaro HORIGUCHI
At Mon, 04 Apr 2016 11:23:34 -0400, Tom Lane wrote in <9798.1459783...@sss.pgh.pa.us> > Amit Langote writes: > > On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane wrote: > >> A related issue, now that I've seen this example, is that

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

2016-04-04 Thread Tom Lane
Amit Langote writes: > On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane 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

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

2016-04-04 Thread Amit Langote
On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane wrote: > Amit Langote writes: >> On 2016/04/04 15:17, Rajkumar Raghuwanshi wrote: >>> * .Observation: *Prepare statement execution plan is not getting changed >>> even after altering foreign table to

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

2016-04-04 Thread Tom Lane
Amit Langote writes: > On 2016/04/04 15:17, Rajkumar Raghuwanshi wrote: >> * .Observation: *Prepare statement execution plan is not getting changed >> even after altering foreign table to point to new schema. > I wonder if performing relcache invalidation upon

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

2016-04-04 Thread Amit Langote
Hi, Thanks for the report. On 2016/04/04 15:17, Rajkumar Raghuwanshi wrote: > Hi, > > I observed below in postgres_fdw > > * .Observation: *Prepare statement execution plan is not getting changed > even after altering foreign table to point to new schema. > [ ... ] > PREPARE stmt_ft AS

[HACKERS] postgres_fdw : altering foreign table not invalidating prepare statement execution plan.

2016-04-04 Thread Rajkumar Raghuwanshi
Hi, I observed below in postgres_fdw * .Observation: *Prepare statement execution plan is not getting changed even after altering foreign table to point to new schema. CREATE EXTENSION postgres_fdw; CREATE SCHEMA s1; create table s1.lt (c1 integer, c2 varchar); insert into s1.lt values (1,