Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-30 Thread Etsuro Fujita
(2018/01/31 4:56), Robert Haas wrote: On Sat, Jan 20, 2018 at 12:20 AM, Tom Lane wrote: It looks like Etsuro-san's proposed patch locks down the choice of plan more tightly, which is probably a reasonable answer. OK, committed. I added a comment along the lines you

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-30 Thread Robert Haas
On Sat, Jan 20, 2018 at 12:20 AM, Tom Lane wrote: > It looks like Etsuro-san's proposed patch locks down the choice of > plan more tightly, which is probably a reasonable answer. OK, committed. I added a comment along the lines you suggested previously, since this no longer

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-19 Thread Tom Lane
Robert Haas writes: > Well, I'm a little stumped. When I do it the way you did it, it fails > with the same error you got: > contrib_regression=# EXPLAIN (VERBOSE, COSTS OFF) > SELECT * FROM ft1, ft2, ft4, ft5 WHERE ft1.c1 = ft2.c1 AND ft1.c1 = ft4.c1 > AND ft1.c1 =

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-19 Thread Robert Haas
On Fri, Jan 19, 2018 at 10:40 AM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Jan 19, 2018 at 1:53 AM, Etsuro Fujita >> wrote: >>> I noticed that this test case added by the patch is not appropriate: >>> because it

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-19 Thread Tom Lane
Robert Haas writes: > On Fri, Jan 19, 2018 at 1:53 AM, Etsuro Fujita > wrote: >> I noticed that this test case added by the patch is not appropriate: >> because it doesn't inject extra Sort nodes into EPQ recheck plans, so it >> works well

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-19 Thread Robert Haas
On Fri, Jan 19, 2018 at 1:53 AM, Etsuro Fujita wrote: > I noticed that this test case added by the patch is not appropriate: > > +-- multi-way join involving multiple merge joins > +EXPLAIN (VERBOSE, COSTS OFF) > +SELECT * FROM ft1, ft2, ft4, ft5 WHERE ft1.c1 = ft2.c1

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-18 Thread Etsuro Fujita
(2018/01/18 15:40), Etsuro Fujita wrote: (2018/01/18 7:09), Robert Haas wrote: On Wed, Jan 17, 2018 at 4:08 PM, Tom Lane wrote: It's debatable perhaps -- I tend to err in the other direction. But likewise, I don't care deeply. Just push it ... Done. I noticed that this

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Etsuro Fujita
(2018/01/18 7:09), Robert Haas wrote: On Wed, Jan 17, 2018 at 4:08 PM, Tom Lane wrote: It's debatable perhaps -- I tend to err in the other direction. But likewise, I don't care deeply. Just push it ... Done. Thanks for taking the time to work on this issue, Robert and

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Robert Haas
On Wed, Jan 17, 2018 at 4:08 PM, Tom Lane wrote: > It's debatable perhaps -- I tend to err in the other direction. > But likewise, I don't care deeply. Just push it ... Done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Tom Lane
Robert Haas writes: > On Wed, Jan 17, 2018 at 3:37 PM, Tom Lane wrote: >> Looks OK to me. Would it be worth annotating the added regression test >> case with a comment that this once caused EPQ-related planning problems? > I tend to think somebody who

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Robert Haas
On Wed, Jan 17, 2018 at 3:37 PM, Tom Lane wrote: > Robert Haas writes: >> Thanks for the review. Updated patch attached. > > Looks OK to me. Would it be worth annotating the added regression test > case with a comment that this once caused EPQ-related

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Robert Haas
On Mon, Jan 15, 2018 at 4:38 PM, Tom Lane wrote: > There *is* a problem with GetExistingLocalJoinPath not honoring its > API spec: it will sometimes return join nests that include remote > joins at levels below the top, as I'd speculated to begin with. > This turns out not to

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Tom Lane
Etsuro Fujita writes: > (2018/01/16 6:38), Tom Lane wrote: >> I'm also still pretty unhappy with the amount of useless planning work >> caused by doing GetExistingLocalJoinPath during path creation. It strikes >> me that we could likely replace the entire thing with

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Etsuro Fujita
(2018/01/16 6:38), Tom Lane wrote: Robert Haas writes: On Mon, Jan 15, 2018 at 12:31 PM, Tom Lane wrote: Hm. Simple is certainly good, but if there's multiple rows coming back during an EPQ recheck then I think we have a performance problem. You

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-17 Thread Etsuro Fujita
(2018/01/16 12:00), Etsuro Fujita wrote: (2018/01/16 11:17), Tom Lane wrote: Etsuro Fujita writes: (2018/01/16 1:47), Robert Haas wrote: Hmm, I was thinking that bar and baz wouldn't be constrained to return just one tuple in that case, but I'm wrong: there would

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-15 Thread Etsuro Fujita
(2018/01/16 11:17), Tom Lane wrote: Etsuro Fujita writes: (2018/01/16 1:47), Robert Haas wrote: Hmm, I was thinking that bar and baz wouldn't be constrained to return just one tuple in that case, but I'm wrong: there would just be one tuple per relation in that

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-15 Thread Tom Lane
Etsuro Fujita writes: > (2018/01/16 1:47), Robert Haas wrote: >> Hmm, I was thinking that bar and baz wouldn't be constrained to return >> just one tuple in that case, but I'm wrong: there would just be one >> tuple per relation in that case. However, that would also

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-15 Thread Etsuro Fujita
(2018/01/16 1:47), Robert Haas wrote: On Mon, Jan 15, 2018 at 3:09 AM, Etsuro Fujita wrote: Yeah, but I don't think the above example is good enough to explain that, because I think the bar/baz join would produce at most one tuple in an EPQ recheck since we would

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-15 Thread Robert Haas
On Mon, Jan 15, 2018 at 12:31 PM, Tom Lane wrote: >> After some thought, it seems that there's a much simpler way that we >> could fix the problem you identified in that original email: if the >> EPQ path isn't properly sorted, have postgres_fdw's >>

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-15 Thread Tom Lane
Robert Haas writes: > I started to look at this patch again today and got cold feet. It > seems to me that this entire design on the posted patch is based on > your remarks in http://postgr.es/m/13242.1481582...@sss.pgh.pa.us -- > # One way that we could make things

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-15 Thread Robert Haas
On Mon, Jan 15, 2018 at 3:09 AM, Etsuro Fujita wrote: > Yeah, but I don't think the above example is good enough to explain that, > because I think the bar/baz join would produce at most one tuple in an EPQ > recheck since we would have only one EPQ tuple for both bar

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-15 Thread Etsuro Fujita
(2018/01/13 6:07), Robert Haas wrote: On Sun, Dec 3, 2017 at 2:56 PM, Tom Lane wrote: I thought some more about this. While it seems clear that we don't actually need to recheck anything within a postgres_fdw foreign join, there's still a problem: we have to be able to

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-12 Thread Robert Haas
On Sun, Dec 3, 2017 at 2:56 PM, Tom Lane wrote: > I thought some more about this. While it seems clear that we don't > actually need to recheck anything within a postgres_fdw foreign join, > there's still a problem: we have to be able to regurgitate the join > row during

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-11 Thread Etsuro Fujita
(2018/01/12 10:41), Thomas Munro wrote: On Mon, Dec 11, 2017 at 8:25 PM, Etsuro Fujita wrote: Now, if you're still super-concerned about this breaking something, we could commit it only to master, where it will have 9 months to bake before it gets released. I

Re: [HACKERS] postgres_fdw bug in 9.6

2018-01-11 Thread Thomas Munro
On Mon, Dec 11, 2017 at 8:25 PM, Etsuro Fujita wrote: >> Now, if you're still super-concerned about this breaking something, we >> could commit it only to master, where it will have 9 months to bake >> before it gets released. I think that's overly conservative, but

Re: [HACKERS] postgres_fdw bug in 9.6

2017-12-10 Thread Etsuro Fujita
(2017/12/09 5:53), Robert Haas wrote: On Sun, Dec 3, 2017 at 2:56 PM, Tom Lane wrote: Not sure where that leaves us for the patch at hand. It feels to me like it's a temporary band-aid for code that we want to get rid of in the not-too-long run. As such, it'd be okay if

Re: [HACKERS] postgres_fdw bug in 9.6

2017-12-08 Thread Robert Haas
On Sun, Dec 3, 2017 at 2:56 PM, Tom Lane wrote: > Not sure where that leaves us for the patch at hand. It feels to me > like it's a temporary band-aid for code that we want to get rid of > in the not-too-long run. As such, it'd be okay if it were smaller, > but it seems

Re: [HACKERS] postgres_fdw bug in 9.6

2017-12-08 Thread Etsuro Fujita
(2017/11/30 23:22), Tom Lane wrote: Etsuro Fujita writes: (2017/11/30 7:32), Tom Lane wrote: the output of the foreign join cannot change during EPQ, since the remote server already locked the rows before returning them. The only thing that can change is the

Re: [HACKERS] postgres_fdw bug in 9.6

2017-12-03 Thread Tom Lane
Robert Haas writes: > But have a look at this: > http://postgr.es/m/561e12d4.7040...@lab.ntt.co.jp > That shows a case where, before > 5fc4c26db5120bd90348b6ee3101fcddfdf54800, a query that required the > foreign table to do an EPQ recheck produced an unambiguously wrong >

Re: [HACKERS] postgres_fdw bug in 9.6

2017-11-30 Thread Robert Haas
On Wed, Nov 29, 2017 at 5:32 PM, Tom Lane wrote: > AFAICT, [1] just demonstrates that nobody had thought about the scenario > up to that point, not that the subsequently chosen solution was a good > one. But have a look at this:

Re: [HACKERS] postgres_fdw bug in 9.6

2017-11-30 Thread Tom Lane
Etsuro Fujita writes: > (2017/11/30 7:32), Tom Lane wrote: >> the output of the foreign join cannot change during EPQ, since the remote >> server already locked the rows before returning them. The only thing that >> can change is the output of the local scan on

Re: [HACKERS] postgres_fdw bug in 9.6

2017-11-30 Thread Etsuro Fujita
(2017/11/30 7:32), Tom Lane wrote: AFAICT, [1] just demonstrates that nobody had thought about the scenario up to that point, not that the subsequently chosen solution was a good one. In that example, LockRows (cost=100.00..101.18 rows=4 width=70) Output: tab.a, tab.b, tab.ctid, foo.*,

Re: [HACKERS] postgres_fdw bug in 9.6

2017-11-29 Thread Michael Paquier
On Thu, Nov 30, 2017 at 7:32 AM, Tom Lane wrote: > [snip] Moving to next CF per the hotness of the topic. -- Michael

Re: [HACKERS] postgres_fdw bug in 9.6

2017-11-28 Thread Ashutosh Bapat
On Tue, Nov 28, 2017 at 11:05 PM, Robert Haas wrote: > On Tue, Nov 28, 2017 at 11:21 AM, Tom Lane wrote: >> In short, we should get rid of all of this expensive and broken logic and >> just make EPQ recheck on a foreign join be a no-op, just as it is

Re: [HACKERS] postgres_fdw bug in 9.6

2017-11-28 Thread Robert Haas
On Tue, Nov 28, 2017 at 11:21 AM, Tom Lane wrote: > In short, we should get rid of all of this expensive and broken logic and > just make EPQ recheck on a foreign join be a no-op, just as it is for a > foreign base table. I'm not sure that it is. What of

Re: [HACKERS] postgres_fdw bug in 9.6

2017-11-28 Thread Tom Lane
I wrote: > Robert Haas writes: >> On Tue, Aug 29, 2017 at 5:14 PM, Tom Lane wrote: >>> We'd definitely need to do things that way in 9.6. I'm not quite sure >>> whether it's too late to adopt the clean solution in v10. >> It probably is now. Are you