diff options
| -rw-r--r-- | src/backend/optimizer/path/joinpath.c | 32 | ||||
| -rw-r--r-- | src/test/regress/expected/equivclass.out | 48 | ||||
| -rw-r--r-- | src/test/regress/sql/equivclass.sql | 34 |
3 files changed, 114 insertions, 0 deletions
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index b0e8c94dfc3..a244300463c 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -25,6 +25,7 @@ #include "optimizer/paths.h" #include "optimizer/placeholder.h" #include "optimizer/planmain.h" +#include "utils/lsyscache.h" #include "utils/typcache.h" /* Hook for plugins to get control in add_paths_to_joinrel() */ @@ -2266,6 +2267,20 @@ hash_inner_and_outer(PlannerInfo *root, if (!clause_sides_match_join(restrictinfo, outerrel, innerrel)) continue; /* no good for these input relations */ + /* + * If clause has the form "inner op outer", check if its operator has + * valid commutator. This is necessary because hashclauses in this + * form will get commuted in createplan.c to put the outer var on the + * left (see get_switched_clauses). This probably shouldn't ever + * fail, since hashable operators ought to have commutators, but be + * paranoid. + * + * The clause being hashjoinable indicates that it's an OpExpr. + */ + if (!restrictinfo->outer_is_left && + !OidIsValid(get_commutator(castNode(OpExpr, restrictinfo->clause)->opno))) + continue; + hashclauses = lappend(hashclauses, restrictinfo); } @@ -2541,6 +2556,23 @@ select_mergejoin_clauses(PlannerInfo *root, } /* + * If clause has the form "inner op outer", check if its operator has + * valid commutator. This is necessary because mergejoin clauses in + * this form will get commuted in createplan.c to put the outer var on + * the left (see get_switched_clauses). This probably shouldn't ever + * fail, since mergejoinable operators ought to have commutators, but + * be paranoid. + * + * The clause being mergejoinable indicates that it's an OpExpr. + */ + if (!restrictinfo->outer_is_left && + !OidIsValid(get_commutator(castNode(OpExpr, restrictinfo->clause)->opno))) + { + have_nonmergeable_joinclause = true; + continue; + } + + /* * Insist that each side have a non-redundant eclass. This * restriction is needed because various bits of the planner expect * that each clause in a merge be associable with some pathkey in a diff --git a/src/test/regress/expected/equivclass.out b/src/test/regress/expected/equivclass.out index 126f7047fed..a328164fe0f 100644 --- a/src/test/regress/expected/equivclass.out +++ b/src/test/regress/expected/equivclass.out @@ -451,3 +451,51 @@ explain (costs off) -- this should not require a sort Filter: (f1 = 'foo'::name) (2 rows) +-- +-- test handling of merge/hash clauses that do not have valid commutators +-- +-- There are not (and should not be) any such operators built into Postgres +-- that are mergejoinable or hashable but have no commutators; so we leverage +-- the alias type 'int8alias1' created in this file to conduct the tests. +-- That's why this test is included here rather than in join.sql. +begin; +create table tbl_nocom(a int8, b int8alias1); +-- check that non-commutable merge clauses do not lead to error +set enable_hashjoin to off; +set enable_mergejoin to on; +explain (costs off) +select * from tbl_nocom t1 full join tbl_nocom t2 on t2.a = t1.b; + QUERY PLAN +-------------------------------------- + Merge Full Join + Merge Cond: (t2.a = t1.b) + -> Sort + Sort Key: t2.a + -> Seq Scan on tbl_nocom t2 + -> Sort + Sort Key: t1.b USING < + -> Seq Scan on tbl_nocom t1 +(8 rows) + +-- check that non-commutable hash clauses do not lead to error +alter operator = (int8, int8alias1) set (hashes); +alter operator family integer_ops using hash add + operator 1 = (int8, int8alias1); +create function hashint8alias1(int8alias1) returns int + strict immutable language internal as 'hashint8'; +alter operator family integer_ops using hash add + function 1 hashint8alias1(int8alias1); +set enable_hashjoin to on; +set enable_mergejoin to off; +explain (costs off) +select * from tbl_nocom t1 full join tbl_nocom t2 on t2.a = t1.b; + QUERY PLAN +-------------------------------------- + Hash Full Join + Hash Cond: (t2.a = t1.b) + -> Seq Scan on tbl_nocom t2 + -> Hash + -> Seq Scan on tbl_nocom t1 +(5 rows) + +abort; diff --git a/src/test/regress/sql/equivclass.sql b/src/test/regress/sql/equivclass.sql index 247b0a31055..28ed7910d01 100644 --- a/src/test/regress/sql/equivclass.sql +++ b/src/test/regress/sql/equivclass.sql @@ -269,3 +269,37 @@ create temp view overview as select f1::information_schema.sql_identifier as sqli, f2 from undername; explain (costs off) -- this should not require a sort select * from overview where sqli = 'foo' order by sqli; + +-- +-- test handling of merge/hash clauses that do not have valid commutators +-- + +-- There are not (and should not be) any such operators built into Postgres +-- that are mergejoinable or hashable but have no commutators; so we leverage +-- the alias type 'int8alias1' created in this file to conduct the tests. +-- That's why this test is included here rather than in join.sql. + +begin; + +create table tbl_nocom(a int8, b int8alias1); + +-- check that non-commutable merge clauses do not lead to error +set enable_hashjoin to off; +set enable_mergejoin to on; +explain (costs off) +select * from tbl_nocom t1 full join tbl_nocom t2 on t2.a = t1.b; + +-- check that non-commutable hash clauses do not lead to error +alter operator = (int8, int8alias1) set (hashes); +alter operator family integer_ops using hash add + operator 1 = (int8, int8alias1); +create function hashint8alias1(int8alias1) returns int + strict immutable language internal as 'hashint8'; +alter operator family integer_ops using hash add + function 1 hashint8alias1(int8alias1); +set enable_hashjoin to on; +set enable_mergejoin to off; +explain (costs off) +select * from tbl_nocom t1 full join tbl_nocom t2 on t2.a = t1.b; + +abort; |