Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-22 Thread Tom Lane
Etsuro Fujita writes: > On 2016/07/21 16:30, Etsuro Fujita wrote: >> One thing I'd like to discuss is GetUserMappingById/GetUserMappingId. >> Though those functions aren't used in any places, I didn't take them >> out, because I thought somebody else would need them

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-21 Thread Etsuro Fujita
On 2016/07/21 16:30, Etsuro Fujita wrote: One thing I'd like to discuss is GetUserMappingById/GetUserMappingId. Though those functions aren't used in any places, I didn't take them out, because I thought somebody else would need them someday. But considering that user mapping OIDs now aren't

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-21 Thread Etsuro Fujita
On 2016/07/16 6:25, Tom Lane wrote: Pushed, after fooling around with it some more so that it would cover the case we discussed where the join could be allowed if we restrict the plan to be executed by the owner of a view used in the query. I left that for future work, but I'm happy to have

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-20 Thread Robert Haas
On Wed, Jul 20, 2016 at 10:15 AM, Tom Lane wrote: > Robert Haas writes: >> Mumble. Why, exactly, was this a good idea? The upside of commit >> 45639a0525a58a2700cf46d4c934d6de78349dac is only that you do fewer >> plan invalidations, but surely that's

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-20 Thread Tom Lane
Robert Haas writes: > Mumble. Why, exactly, was this a good idea? The upside of commit > 45639a0525a58a2700cf46d4c934d6de78349dac is only that you do fewer > plan invalidations, but surely that's not a significant benefit for > most people: user mappings don't change that

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-20 Thread Robert Haas
On Fri, Jul 15, 2016 at 5:25 PM, Tom Lane wrote: > I wrote: >> Etsuro Fujita writes: >>> Here is a patch for that redesign proposed by you; reverts commits >>> fbe5a3fb73102c2cfec114a67943f4474383 and >>>

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-15 Thread Tom Lane
I wrote: > Etsuro Fujita writes: >> Here is a patch for that redesign proposed by you; reverts commits >> fbe5a3fb73102c2cfec114a67943f4474383 and >> 5d4171d1c70edfe3e9be1de9e66603af28e3afe1, adds changes for that redesign >> to the core, and adjusts the

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-15 Thread Tom Lane
Etsuro Fujita writes: > On 2016/07/15 11:48, Tom Lane wrote: >> If we add a mechanism to let us know that the FDW doesn't care, we could >> relax the requirement for such cases. I don't have a strong opinion on >> whether that's worthwhile. It'd depend in part on

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-15 Thread Etsuro Fujita
On 2016/07/15 11:48, Tom Lane wrote: Etsuro Fujita writes: One thing I'm not sure about is: should we insist that a join can be pushed down only if the checkAsUser fields of the relevant RTEs are equal in the case where user mappings are meaningless to the FDW,

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Ashutosh Bapat
On Fri, Jul 15, 2016 at 12:19 AM, Tom Lane wrote: > I wrote: > > I concur with Etsuro-san's dislike for hasForeignJoin; that flag is > > underspecified and doesn't convey nearly enough information. I do not > > think a uses_user_mapping flag is much better. ISTM what should

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Ashutosh Bapat
On Thu, Jul 14, 2016 at 10:02 PM, Tom Lane wrote: > Ashutosh Bapat writes: > > Exactly, for a rare scenario, should we be penalizing large number of > plans > > or just continue to use a previously prepared plan when an optimal plan > has > >

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Tom Lane
Etsuro Fujita writes: > One thing I'm not sure about is: should we insist that a join can be > pushed down only if the checkAsUser fields of the relevant RTEs are > equal in the case where user mappings are meaningless to the FDW, like > file_fdw? If we add a

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Etsuro Fujita
On 2016/07/15 3:49, Tom Lane wrote: On reflection, it seems to me that we've gone wrong by tying planning to equality of user mappings at all, and the best way to get out of this is to not do that. Instead, let's insist that a join can be pushed down only if the checkAsUser fields of the

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Tom Lane
I wrote: > I concur with Etsuro-san's dislike for hasForeignJoin; that flag is > underspecified and doesn't convey nearly enough information. I do not > think a uses_user_mapping flag is much better. ISTM what should happen is > that any time we decide to push down a foreign join, we should

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Tom Lane
Ashutosh Bapat writes: > Exactly, for a rare scenario, should we be penalizing large number of plans > or just continue to use a previously prepared plan when an optimal plan has > become available because of changed condition. I would choose second over > the

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Ashutosh Bapat
On Thu, Jul 14, 2016 at 5:10 PM, Etsuro Fujita wrote: > On 2016/07/13 18:00, Ashutosh Bapat wrote: > >> To fix the first, I'd like to propose (1) replacing the existing >> has_foreign_join flag in the CachedPlan data structure with a new >> flag, say

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-14 Thread Etsuro Fujita
On 2016/07/13 18:00, Ashutosh Bapat wrote: To fix the first, I'd like to propose (1) replacing the existing has_foreign_join flag in the CachedPlan data structure with a new flag, say uses_user_mapping, that indicates whether a cached plan uses any user mapping regardless of

Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-13 Thread Ashutosh Bapat
> Seems odd to me. Both relations use the same user mapping as before, so > the join should be pushed down. > > Another thing I noticed is that the code in plancache.c would invalidate a > cached plan with a foreign join due to user mapping changes even in the > case where user mappings are

[HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-12 Thread Etsuro Fujita
Hi, I noticed odd behavior in invalidating a cached plan with a foreign join due to user mapping changes. Consider: postgres=# create extension postgres_fdw; postgres=# create server loopback foreign data wrapper postgres_fdw options (dbname 'postgres'); postgres=# create user mapping