Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4f11245

Browse files
author
Richard Guo
committed
Check the validity of commutators for merge/hash clauses
When creating merge or hash join plans in createplan.c, the merge orhash clauses may need to get commuted to ensure that the outer var ison the left and the inner var is on the right if they are not alreadyin the expected form. This requires that their operators havecommutators. Failing to find a commutator at this stage would resultin 'ERROR: could not find commutator for operator xxx', with noopportunity to select an alternative plan.Typically, this is not an issue because mergejoinable or hashableoperators are expected to always have valid commutators. But in someartificial cases this assumption may not hold true. Therefore, herein this patch we check the validity of commutators for clauses in theform "inner op outer" when selecting mergejoin/hash clauses, andconsider a clause unusable for the current pair of outer and innerrelations if it lacks a commutator.There are not (and should not be) any such operators built intoPostgres that are mergejoinable or hashable but have no commutators;so we leverage the alias type 'int8alias1' created in equivclass.sqlto build the test case. This is why the test case is included inequivclass.sql rather than in join.sql.Although this is arguably a bug fix, it cannot be reproduced withoutinstalling an incomplete opclass, which is unlikely to happen inpractice, so no back-patch.Reported-by: Alexander PyhalovAuthor: Richard GuoReviewed-by: Tom LaneDiscussion:https://postgr.es/m/c59ec04a2fef94d9ffc35a9b17dfc081@postgrespro.ru
1 parent08b9b9e commit4f11245

File tree

3 files changed

+114
-0
lines changed

3 files changed

+114
-0
lines changed

‎src/backend/optimizer/path/joinpath.c

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include"optimizer/paths.h"
2626
#include"optimizer/placeholder.h"
2727
#include"optimizer/planmain.h"
28+
#include"utils/lsyscache.h"
2829
#include"utils/typcache.h"
2930

3031
/* Hook for plugins to get control in add_paths_to_joinrel() */
@@ -2266,6 +2267,20 @@ hash_inner_and_outer(PlannerInfo *root,
22662267
if (!clause_sides_match_join(restrictinfo,outerrel,innerrel))
22672268
continue;/* no good for these input relations */
22682269

2270+
/*
2271+
* If clause has the form "inner op outer", check if its operator has
2272+
* valid commutator. This is necessary because hashclauses in this
2273+
* form will get commuted in createplan.c to put the outer var on the
2274+
* left (see get_switched_clauses). This probably shouldn't ever
2275+
* fail, since hashable operators ought to have commutators, but be
2276+
* paranoid.
2277+
*
2278+
* The clause being hashjoinable indicates that it's an OpExpr.
2279+
*/
2280+
if (!restrictinfo->outer_is_left&&
2281+
!OidIsValid(get_commutator(castNode(OpExpr,restrictinfo->clause)->opno)))
2282+
continue;
2283+
22692284
hashclauses=lappend(hashclauses,restrictinfo);
22702285
}
22712286

@@ -2540,6 +2555,23 @@ select_mergejoin_clauses(PlannerInfo *root,
25402555
continue;/* no good for these input relations */
25412556
}
25422557

2558+
/*
2559+
* If clause has the form "inner op outer", check if its operator has
2560+
* valid commutator. This is necessary because mergejoin clauses in
2561+
* this form will get commuted in createplan.c to put the outer var on
2562+
* the left (see get_switched_clauses). This probably shouldn't ever
2563+
* fail, since mergejoinable operators ought to have commutators, but
2564+
* be paranoid.
2565+
*
2566+
* The clause being mergejoinable indicates that it's an OpExpr.
2567+
*/
2568+
if (!restrictinfo->outer_is_left&&
2569+
!OidIsValid(get_commutator(castNode(OpExpr,restrictinfo->clause)->opno)))
2570+
{
2571+
have_nonmergeable_joinclause= true;
2572+
continue;
2573+
}
2574+
25432575
/*
25442576
* Insist that each side have a non-redundant eclass. This
25452577
* restriction is needed because various bits of the planner expect

‎src/test/regress/expected/equivclass.out

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,3 +451,51 @@ explain (costs off) -- this should not require a sort
451451
Filter: (f1 = 'foo'::name)
452452
(2 rows)
453453

454+
--
455+
-- test handling of merge/hash clauses that do not have valid commutators
456+
--
457+
-- There are not (and should not be) any such operators built into Postgres
458+
-- that are mergejoinable or hashable but have no commutators; so we leverage
459+
-- the alias type 'int8alias1' created in this file to conduct the tests.
460+
-- That's why this test is included here rather than in join.sql.
461+
begin;
462+
create table tbl_nocom(a int8, b int8alias1);
463+
-- check that non-commutable merge clauses do not lead to error
464+
set enable_hashjoin to off;
465+
set enable_mergejoin to on;
466+
explain (costs off)
467+
select * from tbl_nocom t1 full join tbl_nocom t2 on t2.a = t1.b;
468+
QUERY PLAN
469+
--------------------------------------
470+
Merge Full Join
471+
Merge Cond: (t2.a = t1.b)
472+
-> Sort
473+
Sort Key: t2.a
474+
-> Seq Scan on tbl_nocom t2
475+
-> Sort
476+
Sort Key: t1.b USING <
477+
-> Seq Scan on tbl_nocom t1
478+
(8 rows)
479+
480+
-- check that non-commutable hash clauses do not lead to error
481+
alter operator = (int8, int8alias1) set (hashes);
482+
alter operator family integer_ops using hash add
483+
operator 1 = (int8, int8alias1);
484+
create function hashint8alias1(int8alias1) returns int
485+
strict immutable language internal as 'hashint8';
486+
alter operator family integer_ops using hash add
487+
function 1 hashint8alias1(int8alias1);
488+
set enable_hashjoin to on;
489+
set enable_mergejoin to off;
490+
explain (costs off)
491+
select * from tbl_nocom t1 full join tbl_nocom t2 on t2.a = t1.b;
492+
QUERY PLAN
493+
--------------------------------------
494+
Hash Full Join
495+
Hash Cond: (t2.a = t1.b)
496+
-> Seq Scan on tbl_nocom t2
497+
-> Hash
498+
-> Seq Scan on tbl_nocom t1
499+
(5 rows)
500+
501+
abort;

‎src/test/regress/sql/equivclass.sql

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,3 +269,37 @@ create temp view overview as
269269
select f1::information_schema.sql_identifieras sqli, f2from undername;
270270
explain (costs off)-- this should not require a sort
271271
select*from overviewwhere sqli='foo'order by sqli;
272+
273+
--
274+
-- test handling of merge/hash clauses that do not have valid commutators
275+
--
276+
277+
-- There are not (and should not be) any such operators built into Postgres
278+
-- that are mergejoinable or hashable but have no commutators; so we leverage
279+
-- the alias type 'int8alias1' created in this file to conduct the tests.
280+
-- That's why this test is included here rather than in join.sql.
281+
282+
begin;
283+
284+
createtabletbl_nocom(a int8, b int8alias1);
285+
286+
-- check that non-commutable merge clauses do not lead to error
287+
set enable_hashjoin to off;
288+
set enable_mergejoin toon;
289+
explain (costs off)
290+
select*from tbl_nocom t1 fulljoin tbl_nocom t2ont2.a=t1.b;
291+
292+
-- check that non-commutable hash clauses do not lead to error
293+
alteroperator= (int8, int8alias1)set (hashes);
294+
alteroperator family integer_ops using hash add
295+
operator1= (int8, int8alias1);
296+
createfunctionhashint8alias1(int8alias1) returnsint
297+
strict immutable language internalas'hashint8';
298+
alteroperator family integer_ops using hash add
299+
function1 hashint8alias1(int8alias1);
300+
set enable_hashjoin toon;
301+
set enable_mergejoin to off;
302+
explain (costs off)
303+
select*from tbl_nocom t1 fulljoin tbl_nocom t2ont2.a=t1.b;
304+
305+
abort;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp