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

git projects / postgresql.git / commitdiff

Fix another join removal bug: the check on PlaceHolderVars was wrong.

2010年9月25日 23:03:50 +0000 (19:03 -0400)

The previous coding would decide that join removal was unsafe upon finding
a PlaceHolderVar that needed to be evaluated at the inner rel and then used
above the join. However, this fails to cover the case of PlaceHolderVars
that refer to both the inner rel and some other rels. Per bug report from
Andrus.

src/backend/optimizer/plan/analyzejoins.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/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c

index 97f8839ff2c73f177e4e9b86dc9765672dda58ec..2a913578eba442e95952b79c0a29276012da34f2 100644 (file)

--- a/src/backend/optimizer/plan/analyzejoins.c

+++ b/src/backend/optimizer/plan/analyzejoins.c

@@ -26,6 +26,7 @@

#include "optimizer/pathnode.h"

#include "optimizer/paths.h"

#include "optimizer/planmain.h"

+#include "optimizer/var.h"

/* local functions */

static bool join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo);

@@ -197,16 +198,23 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)

}

- * Similarly check that the inner rel doesn't produce any PlaceHolderVars

- * that will be used above the join.

+ * Similarly check that the inner rel isn't needed by any PlaceHolderVars

+ * that will be used above the join. We only need to fail if such a PHV

+ * actually references some inner-rel attributes; but the correct check

+ * for that is relatively expensive, so we first check against ph_eval_at,

+ * which must mention the inner rel if the PHV uses any inner-rel attrs.

foreach(l, root->placeholder_list)

{

PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);

- if (bms_is_subset(phinfo->ph_eval_at, innerrel->relids) &&

- !bms_is_subset(phinfo->ph_needed, joinrelids))

- return false;

+ if (bms_is_subset(phinfo->ph_needed, joinrelids))

+ continue; /* PHV is not used above the join */

+ if (!bms_overlap(phinfo->ph_eval_at, innerrel->relids))

+ continue; /* it definitely doesn't reference innerrel */

+ if (bms_overlap(pull_varnos((Node *) phinfo->ph_var),

+ innerrel->relids))

+ return false; /* it does reference innerrel */

}

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

index 5299a10ac4e88a164ffa99e4db293a2c1dca7082..795495b14d1bd3449835121d9e1519b22c0d7263 100644 (file)

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

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

@@ -2644,3 +2644,24 @@ SELECT b.* FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0

(1 row)

rollback;

+-- another join removal bug: this is not optimizable, either

+begin;

+create temp table innertab (id int8 primary key, dat1 int8);

+NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "innertab_pkey" for table "innertab"

+insert into innertab values(123, 42);

+SELECT * FROM

+ (SELECT 1 AS x) ss1

+ LEFT JOIN

+ (SELECT q1, q2, COALESCE(dat1, q1) AS y

+ FROM int8_tbl LEFT JOIN innertab ON q2 = id) ss2

+ ON true;

+ x | q1 | q2 | y

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

+ 1 | 123 | 456 | 123

+ 1 | 123 | 4567890123456789 | 123

+ 1 | 4567890123456789 | 123 | 42

+ 1 | 4567890123456789 | 4567890123456789 | 4567890123456789

+ 1 | 4567890123456789 | -4567890123456789 | 4567890123456789

+(5 rows)

+rollback;

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

index ea237f977f1ca7f9a04cf5a051b1c51c3354c00d..b5971b604482f7ea2659bb4925b680fa145eed4e 100644 (file)

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

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

@@ -644,3 +644,18 @@ SELECT * FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0);

SELECT b.* FROM b LEFT JOIN a ON (b.a_id = a.id) WHERE (a.id IS NULL OR a.id > 0);