git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

Undo mistaken tightening in join_is_legal().

2015年8月13日 01:18:45 +0000 (21:18 -0400)

2015年8月13日 01:19:03 +0000 (21:19 -0400)

One of the changes I made in commit 8703059c6b55c427 turns out not to have
been such a good idea: we still need the exception in join_is_legal() that
allows a join if both inputs already overlap the RHS of the special join
we're checking. Otherwise we can miss valid plans, and might indeed fail
to find a plan at all, as in recent report from Andreas Seltenreich.

That code was added way back in commit c17117649b9ae23d, but I failed to
include a regression test case then; my bad. Put it back with a better
explanation, and a test this time. The logic does end up a bit different
than before though: I now believe it's appropriate to make this check
first, thereby allowing such a case whether or not we'd consider the
previous SJ(s) to commute with this one. (Presumably, we already decided
they did; but it was confusing to have this consideration in the middle
of the code that was handling the other case.)

Back-patch to all active branches, like the previous patch.

src/backend/optimizer/path/joinrels.c patch | blob | blame | history

src/test/regress/expected/join.out patch | blob | blame | history

src/test/regress/sql/join.sql patch | blob | blame | history

diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c

index e0103c80431605a14655b56d8a414609d83d558b..b2cc9f07f56d05751bfde08d7a83dcabeb4575a3 100644 (file)

--- a/src/backend/optimizer/path/joinrels.c

+++ b/src/backend/optimizer/path/joinrels.c

@@ -470,11 +470,30 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,

{

* Otherwise, the proposed join overlaps the RHS but isn't a valid

- * implementation of this SJ. It might still be a legal join,

- * however, if we're allowed to associate it into the RHS of this

- * SJ. That means this SJ must be a LEFT join (not SEMI or ANTI,

- * and certainly not FULL) and the proposed join must not overlap

- * the LHS.

+ * implementation of this SJ. But don't panic quite yet: the RHS

+ * violation might have occurred previously, in one or both input

+ * relations, in which case we must have previously decided that

+ * it was OK to commute some other SJ with this one. If we need

+ * to perform this join to finish building up the RHS, rejecting

+ * it could lead to not finding any plan at all. (This can occur

+ * because of the heuristics elsewhere in this file that postpone

+ * clauseless joins: we might not consider doing a clauseless join

+ * within the RHS until after we've performed other, validly

+ * commutable SJs with one or both sides of the clauseless join.)

+ * This consideration boils down to the rule that if both inputs

+ * overlap the RHS, we can allow the join --- they are either

+ * fully within the RHS, or represent previously-allowed joins to

+ * rels outside it.

+ */

+ if (bms_overlap(rel1->relids, sjinfo->min_righthand) &&

+ bms_overlap(rel2->relids, sjinfo->min_righthand))

+ continue; /* assume valid previous violation of RHS */

+ /*

+ * The proposed join could still be legal, but only if we're

+ * allowed to associate it into the RHS of this SJ. That means

+ * this SJ must be a LEFT join (not SEMI or ANTI, and certainly

+ * not FULL) and the proposed join must not overlap the LHS.

if (sjinfo->jointype != JOIN_LEFT ||

bms_overlap(joinrelids, sjinfo->min_lefthand))

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out

index cd4713f5e1723f589c1d616ff86db0c433e1a8bf..eafcd406dc0a5756b9e356e23bbc9b61a76269cd 100644 (file)

--- a/src/test/regress/expected/join.out

+++ b/src/test/regress/expected/join.out

@@ -3563,6 +3563,52 @@ select t1.* from

hi de ho neighbor

(2 rows)

+explain (verbose, costs off)

+select * from

+ text_tbl t1

+ inner join int8_tbl i8

+ on i8.q2 = 456

+ right join text_tbl t2

+ on t1.f1 = 'doh!'

+ left join int4_tbl i4

+ on i8.q1 = i4.f1;

+ QUERY PLAN

+--------------------------------------------------------

+ Nested Loop Left Join

+ Output: t1.f1, i8.q1, i8.q2, t2.f1, i4.f1

+ -> Seq Scan on public.text_tbl t2

+ Output: t2.f1

+ -> Materialize

+ Output: i8.q1, i8.q2, i4.f1, t1.f1

+ -> Nested Loop

+ Output: i8.q1, i8.q2, i4.f1, t1.f1

+ -> Nested Loop Left Join

+ Output: i8.q1, i8.q2, i4.f1

+ Join Filter: (i8.q1 = i4.f1)

+ -> Seq Scan on public.int8_tbl i8

+ Output: i8.q1, i8.q2

+ Filter: (i8.q2 = 456)

+ -> Seq Scan on public.int4_tbl i4

+ Output: i4.f1

+ -> Seq Scan on public.text_tbl t1

+ Output: t1.f1

+ Filter: (t1.f1 = 'doh!'::text)

+(19 rows)

+select * from

+ text_tbl t1

+ inner join int8_tbl i8

+ on i8.q2 = 456

+ right join text_tbl t2

+ on t1.f1 = 'doh!'

+ left join int4_tbl i4

+ on i8.q1 = i4.f1;

+ f1 | q1 | q2 | f1 | f1

+------+-----+-----+-------------------+----

+ doh! | 123 | 456 | doh! |

+ doh! | 123 | 456 | hi de ho neighbor |

+(2 rows)

-- test ability to push constants through outer join clauses

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql

index 2b9bd20bc6afcc1be399e6c2f3df78007b7daef5..c9f34aa4e39546ccb97d46b616287cba996cd9fd 100644 (file)

--- a/src/test/regress/sql/join.sql

+++ b/src/test/regress/sql/join.sql

@@ -1108,6 +1108,25 @@ select t1.* from

left join int4_tbl i4

on (i8.q2 = i4.f1);

+explain (verbose, costs off)

+select * from

+ text_tbl t1

+ inner join int8_tbl i8

+ on i8.q2 = 456

+ right join text_tbl t2

+ on t1.f1 = 'doh!'

+ left join int4_tbl i4

+ on i8.q1 = i4.f1;

+select * from

+ text_tbl t1

+ inner join int8_tbl i8

+ on i8.q2 = 456

+ right join text_tbl t2

+ on t1.f1 = 'doh!'

+ left join int4_tbl i4

+ on i8.q1 = i4.f1;

-- test ability to push constants through outer join clauses

This is the main PostgreSQL git repository.

RSS Atom