From a43b190e3c7185e42c7329358c2038cf6d86c5a4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 19 Jul 2009 20:32:48 +0000 Subject: [PATCH] Fix a thinko in join_is_legal: when we decide we can implement a semijoin by unique-ifying the RHS and then inner-joining to some other relation, that is not grounds for violating the RHS of some other outer join. Noticed while regression-testing new GEQO code, which will blindly follow any path that join_is_legal says is legal, and then complain later if that leads to a dead end. I'm not certain that this can result in any visible failure in 8.4: the mistake may always be masked by the fact that subsequent attempts to join the rest of the RHS of the other join will fail. But I'm not certain it can't, either, and it's definitely not operating as intended. So back-patch. The added regression test depends on the new no-failures-allowed logic that I'm about to commit in GEQO, so no point back-patching that. --- src/backend/optimizer/path/joinrels.c | 15 ++++++++++++--- src/test/regress/expected/join.out | 14 ++++++++++++++ src/test/regress/sql/join.sql | 10 ++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index 9e6f57f4c9..350fde29c2 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/joinrels.c,v 1.100 2009/06/11 14:48:59 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/joinrels.c,v 1.101 2009/07/19 20:32:48 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -349,6 +349,7 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, { SpecialJoinInfo *match_sjinfo; bool reversed; + bool unique_ified; bool is_valid_inner; ListCell *l; @@ -366,6 +367,7 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, */ match_sjinfo = NULL; reversed = false; + unique_ified = false; is_valid_inner = true; foreach(l, root->join_info_list) @@ -450,6 +452,7 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, return false; /* invalid join path */ match_sjinfo = sjinfo; reversed = false; + unique_ified = true; } else if (sjinfo->jointype == JOIN_SEMI && bms_equal(sjinfo->syn_righthand, rel1->relids) && @@ -461,6 +464,7 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, return false; /* invalid join path */ match_sjinfo = sjinfo; reversed = true; + unique_ified = true; } else { @@ -510,8 +514,13 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, } } - /* Fail if violated some SJ's RHS and didn't match to another SJ */ - if (match_sjinfo == NULL && !is_valid_inner) + /* + * Fail if violated some SJ's RHS and didn't match to another SJ. + * However, "matching" to a semijoin we are implementing by + * unique-ification doesn't count (think: it's really an inner join). + */ + if (!is_valid_inner && + (match_sjinfo == NULL || unique_ified)) return false; /* invalid join path */ /* Otherwise, it's a valid join */ diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index a7b7b73ad6..afb4f974e7 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2150,6 +2150,20 @@ select count(*) from tenk1 x where 1 (1 row) +-- try that with GEQO too +begin; +set geqo = on; +set geqo_threshold = 2; +select count(*) from tenk1 x where + x.unique1 in (select a.f1 from int4_tbl a,float8_tbl b where a.f1=b.f1) and + x.unique1 = 0 and + x.unique1 in (select aa.f1 from int4_tbl aa,float8_tbl bb where aa.f1=bb.f1); + count +------- + 1 +(1 row) + +rollback; -- -- Clean up -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 29992ced0c..123d1f3015 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -343,6 +343,16 @@ select count(*) from tenk1 x where x.unique1 = 0 and x.unique1 in (select aa.f1 from int4_tbl aa,float8_tbl bb where aa.f1=bb.f1); +-- try that with GEQO too +begin; +set geqo = on; +set geqo_threshold = 2; +select count(*) from tenk1 x where + x.unique1 in (select a.f1 from int4_tbl a,float8_tbl b where a.f1=b.f1) and + x.unique1 = 0 and + x.unique1 in (select aa.f1 from int4_tbl aa,float8_tbl bb where aa.f1=bb.f1); +rollback; + -- -- Clean up -- 2.40.0