Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Fri, May 13, 2016 at 10:14 AM, Robert Haas wrote: > On Wed, Mar 23, 2016 at 12:53 PM, Robert Haas > wrote: > > On Wed, Mar 23, 2016 at 5:24 AM, Rajkumar Raghuwanshi > > wrote: > >> Thanks Ashutosh for the patch. I have apply and retested it, now not > getting > >> server crash. > > > > Thanks for the report and the testing. I have committed the patch. > > So it's been pointed out to me off-list that I have committed a > different patch than the one I intended to commit - that is, the one > Michael sent, not the one Ashutosh sent. Oops. > > Reverting Michael's patch and applying Ashutosh's doesn't work any > more due to conflicts in the regression tests. And in re-rereviewing > Ashutosh's patch I came to feel like the comments in this area needed > a lot more work. > Thanks for improving comments. Those are much better than mine. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
Thanks for the commit. I have tested it again. Not getting server crash now. Thanks & Regards, Rajkumar Raghuwanshi QMG, EnterpriseDB Corporation On Mon, May 16, 2016 at 9:38 PM, Robert Haas wrote: > On Fri, May 13, 2016 at 6:40 PM, Michael Paquier > wrote: > > On Fri, May 13, 2016 at 11:14 PM, Robert Haas > wrote: > >> So, barring objections, I intend to apply the attached fixup patch, > >> which replaces Michael's logic with Ashutosh's logic and rewrites the > >> comments such to be much more explicit. > > > > Re-oops. I didn't check what was committed to be honest. And it should > > not have been my version, definitely. > > OK, committed. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Fri, May 13, 2016 at 6:40 PM, Michael Paquier wrote: > On Fri, May 13, 2016 at 11:14 PM, Robert Haas wrote: >> So, barring objections, I intend to apply the attached fixup patch, >> which replaces Michael's logic with Ashutosh's logic and rewrites the >> comments such to be much more explicit. > > Re-oops. I didn't check what was committed to be honest. And it should > not have been my version, definitely. OK, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Fri, May 13, 2016 at 11:14 PM, Robert Haas wrote: > So, barring objections, I intend to apply the attached fixup patch, > which replaces Michael's logic with Ashutosh's logic and rewrites the > comments such to be much more explicit. Re-oops. I didn't check what was committed to be honest. And it should not have been my version, definitely. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Wed, Mar 23, 2016 at 12:53 PM, Robert Haas wrote: > On Wed, Mar 23, 2016 at 5:24 AM, Rajkumar Raghuwanshi > wrote: >> Thanks Ashutosh for the patch. I have apply and retested it, now not getting >> server crash. > > Thanks for the report and the testing. I have committed the patch. So it's been pointed out to me off-list that I have committed a different patch than the one I intended to commit - that is, the one Michael sent, not the one Ashutosh sent. Oops. Reverting Michael's patch and applying Ashutosh's doesn't work any more due to conflicts in the regression tests. And in re-rereviewing Ashutosh's patch I came to feel like the comments in this area needed a lot more work. So, barring objections, I intend to apply the attached fixup patch, which replaces Michael's logic with Ashutosh's logic and rewrites the comments such to be much more explicit. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company From 5de87782f3fb5898edbdd870e53f08f5e83ddbe5 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 12 May 2016 14:46:08 -0400 Subject: [PATCH] postgres_fdw: Fix the fix for crash when pushing down multiple joins. Commit 3151f16e1874db82ed85a005dac15368903ca9fb was intended to be a commit of a patch from Ashutosh Bapat, but instead I mistakenly committed an earlier version from Michael Paquier (because both patches were submitted with the same filename, and I confused them). Michael's patch fixes the crash but doesn't actually implement the correct test. Repair the incorrect logic, and also expand the comments considerably so that this is all more clear. Ashutosh Bapat and Robert Haas --- contrib/postgres_fdw/expected/postgres_fdw.out | 68 ++ contrib/postgres_fdw/postgres_fdw.c| 36 -- contrib/postgres_fdw/sql/postgres_fdw.sql | 10 3 files changed, 110 insertions(+), 4 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 698aa8f..1de0bc4 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -478,6 +478,74 @@ SELECT t1."C 1" FROM "S 1"."T 1" t1 left join ft1 t2 join ft2 t3 on (t2.c1 = t3. 110 (10 rows) +-- Test similar to above, except that the full join prevents any equivalence +-- classes from being merged. This produces single relation equivalence classes +-- included in join restrictions. +EXPLAIN (COSTS false, VERBOSE) + SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 left join ft1 t2 full join ft2 t3 on (t2.c1 = t3.c1) on (t3.c1 = t1."C 1") OFFSET 100 LIMIT 10; +QUERY PLAN +-- + Limit + Output: t1."C 1", t2.c1, t3.c1 + -> Merge Right Join + Output: t1."C 1", t2.c1, t3.c1 + Merge Cond: (t3.c1 = t1."C 1") + -> Foreign Scan + Output: t3.c1, t2.c1 + Relations: (public.ft2 t3) LEFT JOIN (public.ft1 t2) + Remote SQL: SELECT r3."C 1", r2."C 1" FROM ("S 1"."T 1" r3 LEFT JOIN "S 1"."T 1" r2 ON (((r2."C 1" = r3."C 1" ORDER BY r3."C 1" ASC NULLS LAST + -> Index Only Scan using t1_pkey on "S 1"."T 1" t1 + Output: t1."C 1" +(11 rows) + +SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 left join ft1 t2 full join ft2 t3 on (t2.c1 = t3.c1) on (t3.c1 = t1."C 1") OFFSET 100 LIMIT 10; + C 1 | c1 | c1 +-+-+- + 101 | 101 | 101 + 102 | 102 | 102 + 103 | 103 | 103 + 104 | 104 | 104 + 105 | 105 | 105 + 106 | 106 | 106 + 107 | 107 | 107 + 108 | 108 | 108 + 109 | 109 | 109 + 110 | 110 | 110 +(10 rows) + +-- Test similar to above with all full outer joins +EXPLAIN (COSTS false, VERBOSE) + SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 full join ft1 t2 full join ft2 t3 on (t2.c1 = t3.c1) on (t3.c1 = t1."C 1") OFFSET 100 LIMIT 10; +QUERY PLAN +-- + Limit + Output: t1."C 1", t2.c1, t3.c1 + -> Merge Full Join + Output: t1."C 1", t2.c1, t3.c1 + Merge Cond: (t3.c1 = t1."C 1") + -> Foreign Scan + Output: t2.c1, t3.c1 + Relations: (public.ft1 t2) FULL JOIN (public.ft2 t3) + Remote SQL: SELECT r2."C 1", r3."C 1" FROM ("S 1"."T 1" r2 FULL JOIN "S 1"."T 1" r3 ON (((r2."C 1" = r3."C 1" ORDER BY r3."C 1" ASC NULLS LAST + -> Index Only Scan using t1_pkey on "S 1"."T 1" t1 +
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
> > Thanks for the report and the testing. I have committed the patch. > > Thanks. > Cool, I have refreshed the wiki page for open items accordingly. > Thanks. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Thu, Mar 24, 2016 at 1:53 AM, Robert Haas wrote: > On Wed, Mar 23, 2016 at 5:24 AM, Rajkumar Raghuwanshi > wrote: >> Thanks Ashutosh for the patch. I have apply and retested it, now not getting >> server crash. > > Thanks for the report and the testing. I have committed the patch. Cool, I have refreshed the wiki page for open items accordingly. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Wed, Mar 23, 2016 at 5:24 AM, Rajkumar Raghuwanshi wrote: > Thanks Ashutosh for the patch. I have apply and retested it, now not getting > server crash. Thanks for the report and the testing. I have committed the patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Mon, Mar 21, 2016 at 11:32 PM, Ashutosh Bapat wrote: >> In get_useful_ecs_for_relation, it seems to me that this assertion >> should be removed and replaces by an actual check because even if >> right_ec and left_ec are initialized, we cannot be sure that ec_relids >> contains the relations specified: >> /* >> * restrictinfo->mergeopfamilies != NIL is sufficient to guarantee >> * that left_ec and right_ec will be initialized, per comments in >> * distribute_qual_to_rels, and rel->joininfo should only contain >> ECs >> * where this relation appears on one side or the other. >> */ >> if (bms_is_subset(relids, restrictinfo->right_ec->ec_relids)) >> useful_eclass_list = >> list_append_unique_ptr(useful_eclass_list, >> >> restrictinfo->right_ec); >> else >> { >> Assert(bms_is_subset(relids, >> restrictinfo->left_ec->ec_relids)); >> useful_eclass_list = >> list_append_unique_ptr(useful_eclass_list, >> >> restrictinfo->left_ec); >> } > > An EC covers all the relations covered by all the equivalence members it > contains. In case of mergejoinable clause for outer join, EC may cover just > a single relation whose column appears on either side of the clause. In this > case, bms_is_subset() for a given join relation covering single relation in > EC will be false. So, we have to use bms_overlap() instead of > bms_is_subset(). The caller get_useful_pathkeys_for_rel() extracts the > equivalence member (if any), which is entirely covered by the given > relation. Otherwise, you are correct that we have to convert the assertion > into a condition. I have added comments in get_useful_ecs_for_relation() > explaining, why. Ah, OK. Thanks for the explanation. This code is fixing the problem for me as well here. (note to self: study more the planner code). -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
Thanks Ashutosh for the patch. I have apply and retested it, now not getting server crash. Thanks & Regards, Rajkumar Raghuwanshi QMG, EnterpriseDB Corporation On Mon, Mar 21, 2016 at 8:02 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > Thanks Michael for looking into this. > > > >> In get_useful_ecs_for_relation, it seems to me that this assertion >> should be removed and replaces by an actual check because even if >> right_ec and left_ec are initialized, we cannot be sure that ec_relids >> contains the relations specified: >> /* >> * restrictinfo->mergeopfamilies != NIL is sufficient to guarantee >> * that left_ec and right_ec will be initialized, per comments in >> * distribute_qual_to_rels, and rel->joininfo should only contain >> ECs >> * where this relation appears on one side or the other. >> */ >> if (bms_is_subset(relids, restrictinfo->right_ec->ec_relids)) >> useful_eclass_list = >> list_append_unique_ptr(useful_eclass_list, >> >> restrictinfo->right_ec); >> else >> { >> Assert(bms_is_subset(relids, >> restrictinfo->left_ec->ec_relids)); >> useful_eclass_list = >> list_append_unique_ptr(useful_eclass_list, >> >> restrictinfo->left_ec); >> } >> > > An EC covers all the relations covered by all the equivalence members it > contains. In case of mergejoinable clause for outer join, EC may cover just > a single relation whose column appears on either side of the clause. In > this case, bms_is_subset() for a given join relation covering single > relation in EC will be false. So, we have to use bms_overlap() instead of > bms_is_subset(). The caller get_useful_pathkeys_for_rel() extracts the > equivalence member (if any), which is entirely covered by the given > relation. Otherwise, you are correct that we have to convert the assertion > into a condition. I have added comments in get_useful_ecs_for_relation() > explaining, why. > > See for example the attached (with more tests including combinations >> of joins, and three-table joins). I have added an open item for 9.6 on >> the wiki. >> > > Thanks for those tests. Actually, that code is relevant for joins which > can not be pushed down to the foreign server. For such joins we try to add > pathkeys which will help merge joins. I have included the relevant tests > rewriting them to use local tables, so that the entire join is not pushed > down to the foreign server. > > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company >
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
Thanks Michael for looking into this. > In get_useful_ecs_for_relation, it seems to me that this assertion > should be removed and replaces by an actual check because even if > right_ec and left_ec are initialized, we cannot be sure that ec_relids > contains the relations specified: > /* > * restrictinfo->mergeopfamilies != NIL is sufficient to guarantee > * that left_ec and right_ec will be initialized, per comments in > * distribute_qual_to_rels, and rel->joininfo should only contain > ECs > * where this relation appears on one side or the other. > */ > if (bms_is_subset(relids, restrictinfo->right_ec->ec_relids)) > useful_eclass_list = list_append_unique_ptr(useful_eclass_list, > > restrictinfo->right_ec); > else > { > Assert(bms_is_subset(relids, > restrictinfo->left_ec->ec_relids)); > useful_eclass_list = list_append_unique_ptr(useful_eclass_list, > > restrictinfo->left_ec); > } > An EC covers all the relations covered by all the equivalence members it contains. In case of mergejoinable clause for outer join, EC may cover just a single relation whose column appears on either side of the clause. In this case, bms_is_subset() for a given join relation covering single relation in EC will be false. So, we have to use bms_overlap() instead of bms_is_subset(). The caller get_useful_pathkeys_for_rel() extracts the equivalence member (if any), which is entirely covered by the given relation. Otherwise, you are correct that we have to convert the assertion into a condition. I have added comments in get_useful_ecs_for_relation() explaining, why. See for example the attached (with more tests including combinations > of joins, and three-table joins). I have added an open item for 9.6 on > the wiki. > Thanks for those tests. Actually, that code is relevant for joins which can not be pushed down to the foreign server. For such joins we try to add pathkeys which will help merge joins. I have included the relevant tests rewriting them to use local tables, so that the entire join is not pushed down to the foreign server. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index a7f32f3..d38ff86 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -471,20 +471,88 @@ SELECT t1."C 1" FROM "S 1"."T 1" t1 left join ft1 t2 join ft2 t3 on (t2.c1 = t3. 103 104 105 106 107 108 109 110 (10 rows) +-- Test similar to above, except that the full join prevents any equivalence +-- classes from being merged. This produces single relation equivalence classes +-- included in join restrictions. +EXPLAIN (COSTS false, VERBOSE) + SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 left join ft1 t2 full join ft2 t3 on (t2.c1 = t3.c1) on (t3.c1 = t1."C 1") OFFSET 100 LIMIT 10; +QUERY PLAN +-- + Limit + Output: t1."C 1", t2.c1, t3.c1 + -> Merge Right Join + Output: t1."C 1", t2.c1, t3.c1 + Merge Cond: (t3.c1 = t1."C 1") + -> Foreign Scan + Output: t3.c1, t2.c1 + Relations: (public.ft2 t3) LEFT JOIN (public.ft1 t2) + Remote SQL: SELECT r3."C 1", r2."C 1" FROM ("S 1"."T 1" r3 LEFT JOIN "S 1"."T 1" r2 ON (((r2."C 1" = r3."C 1" ORDER BY r3."C 1" ASC NULLS LAST + -> Index Only Scan using t1_pkey on "S 1"."T 1" t1 + Output: t1."C 1" +(11 rows) + +SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 left join ft1 t2 full join ft2 t3 on (t2.c1 = t3.c1) on (t3.c1 = t1."C 1") OFFSET 100 LIMIT 10; + C 1 | c1 | c1 +-+-+- + 101 | 101 | 101 + 102 | 102 | 102 + 103 | 103 | 103 + 104 | 104 | 104 + 105 | 105 | 105 + 106 | 106 | 106 + 107 | 107 | 107 + 108 | 108 | 108 + 109 | 109 | 109 + 110 | 110 | 110 +(10 rows) + +-- Test similar to above with all full outer joins +EXPLAIN (COSTS false, VERBOSE) + SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 full join ft1 t2 full join ft2 t3 on (t2.c1 = t3.c1) on (t3.c1 = t1."C 1") OFFSET 100 LIMIT 10; +QUERY PLAN +-- + Limit + Output: t1."C 1", t2.c1, t3.c1 + -> Merge Full Join + Output: t1."C 1", t2.c1, t3.c1 + Merge Cond: (t3.c
Re: [HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
On Fri, Mar 18, 2016 at 7:22 PM, Rajkumar Raghuwanshi wrote: > Hi, > > I am testing postgres_fdw join pushdown feature for PostgreSQL 9.6 DB, and I > observed below issue. > > Observation: If do a left outer join on foreign tables more than two times. > It is causing the server crash. > > Added below statement in contrib/postgres_fdw/postgres_fdw.sql and ran make > check, did a server crash > > -- left outer join three tables > EXPLAIN (COSTS false, VERBOSE) > SELECT t1.c1,t2.c1,t3.c1 FROM ft2 t1 LEFT JOIN ft2 t2 ON (t1.c1 = t2.c1) > LEFT JOIN ft2 t3 ON (t2.c1 = t3.c1) OFFSET 10 LIMIT 10; > server closed the connection unexpectedly > This probably means the server terminated abnormally > before or while processing the request. > connection to server was lost > > Facing the same crash while doing left outer join, right outer join or > combination of left-right outer joins for three tables and one local and two > foreign tables. In get_useful_ecs_for_relation, it seems to me that this assertion should be removed and replaces by an actual check because even if right_ec and left_ec are initialized, we cannot be sure that ec_relids contains the relations specified: /* * restrictinfo->mergeopfamilies != NIL is sufficient to guarantee * that left_ec and right_ec will be initialized, per comments in * distribute_qual_to_rels, and rel->joininfo should only contain ECs * where this relation appears on one side or the other. */ if (bms_is_subset(relids, restrictinfo->right_ec->ec_relids)) useful_eclass_list = list_append_unique_ptr(useful_eclass_list, restrictinfo->right_ec); else { Assert(bms_is_subset(relids, restrictinfo->left_ec->ec_relids)); useful_eclass_list = list_append_unique_ptr(useful_eclass_list, restrictinfo->left_ec); } See for example the attached (with more tests including combinations of joins, and three-table joins). I have added an open item for 9.6 on the wiki. -- Michael postgres_fdw_join_fix-v1.patch Description: binary/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Postgres_fdw join pushdown - getting server crash in left outer join of three table
Hi, I am testing postgres_fdw join pushdown feature for PostgreSQL 9.6 DB, and I observed below issue. *Observation:* If do a left outer join on foreign tables more than two times. It is causing the server crash. Added below statement in contrib/postgres_fdw/postgres_fdw.sql and ran make check, did a server crash -- left outer join three tables EXPLAIN (COSTS false, VERBOSE) SELECT t1.c1,t2.c1,t3.c1 FROM ft2 t1 LEFT JOIN ft2 t2 ON (t1.c1 = t2.c1) LEFT JOIN ft2 t3 ON (t2.c1 = t3.c1) OFFSET 10 LIMIT 10; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. connection to server was lost Facing the same crash while doing left outer join, right outer join or combination of left-right outer joins for three tables and one local and two foreign tables. Thanks & Regards, Rajkumar Raghuwanshi QMG, EnterpriseDB Corporation