Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit d7a6a04

Browse files
committed
Fix planner to restore its previous level of intelligence about pushing
constants through full joins, as in select * from tenk1 a full join tenk1 b using (unique1) where unique1 = 42; which should generate a fairly cheap plan where we apply the constraint unique1 = 42 in each relation scan. This had been broken by my patch of 2008-06-27, which is now reverted in favor of a more invasive but hopefully less incorrect approach. That patch was meant to prevent incorrect extraction of OR'd indexclauses from OR conditions above an outer join. To do that correctly we need more information than the outerjoin_delay flag can provide, so add a nullable_relids field to RestrictInfo that records exactly which relations are nulled by outer joins that are underneath a particular qual clause. A side benefit is that we can make the test in create_or_index_quals more specific: it is now smart enough to extract an OR'd indexclause into the outer side of an outer join, even though it must not do so in the inner side. The old coding couldn't distinguish these cases so it could not do either.
1 parent c5593d5 commit d7a6a04

File tree

9 files changed

+132
-68
lines changed

9 files changed

+132
-68
lines changed

src/backend/nodes/copyfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* Portions Copyright (c) 1994, Regents of the University of California
1616
*
1717
* IDENTIFICATION
18-
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.429 2009/04/05 19:59:39 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.430 2009/04/16 20:42:16 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -1606,6 +1606,7 @@ _copyRestrictInfo(RestrictInfo *from)
16061606
COPY_SCALAR_FIELD(pseudoconstant);
16071607
COPY_BITMAPSET_FIELD(clause_relids);
16081608
COPY_BITMAPSET_FIELD(required_relids);
1609+
COPY_BITMAPSET_FIELD(nullable_relids);
16091610
COPY_BITMAPSET_FIELD(left_relids);
16101611
COPY_BITMAPSET_FIELD(right_relids);
16111612
COPY_NODE_FIELD(orclause);

src/backend/nodes/equalfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* Portions Copyright (c) 1994, Regents of the University of California
2323
*
2424
* IDENTIFICATION
25-
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.352 2009/04/05 19:59:40 tgl Exp $
25+
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.353 2009/04/16 20:42:16 tgl Exp $
2626
*
2727
*-------------------------------------------------------------------------
2828
*/
@@ -759,6 +759,7 @@ _equalRestrictInfo(RestrictInfo *a, RestrictInfo *b)
759759
COMPARE_SCALAR_FIELD(is_pushed_down);
760760
COMPARE_SCALAR_FIELD(outerjoin_delayed);
761761
COMPARE_BITMAPSET_FIELD(required_relids);
762+
COMPARE_BITMAPSET_FIELD(nullable_relids);
762763

763764
/*
764765
* We ignore all the remaining fields, since they may not be set yet, and

src/backend/nodes/outfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.358 2009/04/05 19:59:40 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.359 2009/04/16 20:42:16 tgl Exp $
1212
*
1313
* NOTES
1414
* Every node type that can appear in stored rules' parsetrees *must*
@@ -1613,6 +1613,7 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
16131613
WRITE_BOOL_FIELD(pseudoconstant);
16141614
WRITE_BITMAPSET_FIELD(clause_relids);
16151615
WRITE_BITMAPSET_FIELD(required_relids);
1616+
WRITE_BITMAPSET_FIELD(nullable_relids);
16161617
WRITE_BITMAPSET_FIELD(left_relids);
16171618
WRITE_BITMAPSET_FIELD(right_relids);
16181619
WRITE_NODE_FIELD(orclause);

src/backend/optimizer/path/indxpath.c

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.238 2009/03/11 03:32:22 tgl Exp $
12+
* $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.239 2009/04/16 20:42:16 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -2325,11 +2325,7 @@ expand_indexqual_conditions(IndexOptInfo *index, List *clausegroups)
23252325
if (boolqual)
23262326
{
23272327
resultquals = lappend(resultquals,
2328-
make_restrictinfo(boolqual,
2329-
true,
2330-
false,
2331-
false,
2332-
NULL));
2328+
make_simple_restrictinfo(boolqual));
23332329
continue;
23342330
}
23352331
}
@@ -2360,11 +2356,7 @@ expand_indexqual_conditions(IndexOptInfo *index, List *clausegroups)
23602356
{
23612357
Assert(index->amsearchnulls);
23622358
resultquals = lappend(resultquals,
2363-
make_restrictinfo(clause,
2364-
true,
2365-
false,
2366-
false,
2367-
NULL));
2359+
make_simple_restrictinfo(clause));
23682360
}
23692361
else
23702362
elog(ERROR, "unsupported indexqual type: %d",
@@ -2737,7 +2729,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
27372729
matching_cols);
27382730
rc->rargs = list_truncate((List *) copyObject(clause->rargs),
27392731
matching_cols);
2740-
return make_restrictinfo((Expr *) rc, true, false, false, NULL);
2732+
return make_simple_restrictinfo((Expr *) rc);
27412733
}
27422734
else
27432735
{
@@ -2746,7 +2738,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
27462738
opexpr = make_opclause(linitial_oid(new_ops), BOOLOID, false,
27472739
copyObject(linitial(clause->largs)),
27482740
copyObject(linitial(clause->rargs)));
2749-
return make_restrictinfo(opexpr, true, false, false, NULL);
2741+
return make_simple_restrictinfo(opexpr);
27502742
}
27512743
}
27522744

@@ -2832,7 +2824,7 @@ prefix_quals(Node *leftop, Oid opfamily,
28322824
elog(ERROR, "no = operator for opfamily %u", opfamily);
28332825
expr = make_opclause(oproid, BOOLOID, false,
28342826
(Expr *) leftop, (Expr *) prefix_const);
2835-
result = list_make1(make_restrictinfo(expr, true, false, false, NULL));
2827+
result = list_make1(make_simple_restrictinfo(expr));
28362828
return result;
28372829
}
28382830

@@ -2847,7 +2839,7 @@ prefix_quals(Node *leftop, Oid opfamily,
28472839
elog(ERROR, "no >= operator for opfamily %u", opfamily);
28482840
expr = make_opclause(oproid, BOOLOID, false,
28492841
(Expr *) leftop, (Expr *) prefix_const);
2850-
result = list_make1(make_restrictinfo(expr, true, false, false, NULL));
2842+
result = list_make1(make_simple_restrictinfo(expr));
28512843

28522844
/*-------
28532845
* If we can create a string larger than the prefix, we can say
@@ -2864,8 +2856,7 @@ prefix_quals(Node *leftop, Oid opfamily,
28642856
{
28652857
expr = make_opclause(oproid, BOOLOID, false,
28662858
(Expr *) leftop, (Expr *) greaterstr);
2867-
result = lappend(result,
2868-
make_restrictinfo(expr, true, false, false, NULL));
2859+
result = lappend(result, make_simple_restrictinfo(expr));
28692860
}
28702861

28712862
return result;
@@ -2928,7 +2919,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opfamily, Datum rightop)
29282919
(Expr *) leftop,
29292920
(Expr *) makeConst(datatype, -1, -1, opr1right,
29302921
false, false));
2931-
result = list_make1(make_restrictinfo(expr, true, false, false, NULL));
2922+
result = list_make1(make_simple_restrictinfo(expr));
29322923

29332924
/* create clause "key <= network_scan_last( rightop )" */
29342925

@@ -2943,8 +2934,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opfamily, Datum rightop)
29432934
(Expr *) leftop,
29442935
(Expr *) makeConst(datatype, -1, -1, opr2right,
29452936
false, false));
2946-
result = lappend(result,
2947-
make_restrictinfo(expr, true, false, false, NULL));
2937+
result = lappend(result, make_simple_restrictinfo(expr));
29482938

29492939
return result;
29502940
}

src/backend/optimizer/path/orindxpath.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.88 2009/02/15 20:16:21 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.89 2009/04/16 20:42:16 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -89,20 +89,26 @@ create_or_index_quals(PlannerInfo *root, RelOptInfo *rel)
8989
ListCell *i;
9090

9191
/*
92-
* Find potentially interesting OR joinclauses. Note we must ignore any
93-
* joinclauses that are marked outerjoin_delayed or !is_pushed_down,
94-
* because they cannot be pushed down to the per-relation level due to
95-
* outer-join rules. (XXX in some cases it might be possible to allow
96-
* this, but it would require substantially more bookkeeping about where
97-
* the clause came from.)
92+
* Find potentially interesting OR joinclauses.
93+
*
94+
* We must ignore clauses for which the target rel is in nullable_relids;
95+
* that means there's an outer join below the clause and so it can't be
96+
* enforced at the relation scan level.
97+
*
98+
* We must also ignore clauses that are marked !is_pushed_down (ie they
99+
* are themselves outer-join clauses). It would be safe to extract an
100+
* index condition from such a clause if we are within the nullable rather
101+
* than the non-nullable side of its join, but we haven't got enough
102+
* context here to tell which applies. OR clauses in outer-join quals
103+
* aren't exactly common, so we'll let that case go unoptimized for now.
98104
*/
99105
foreach(i, rel->joininfo)
100106
{
101107
RestrictInfo *rinfo = (RestrictInfo *) lfirst(i);
102108

103109
if (restriction_is_or_clause(rinfo) &&
104110
rinfo->is_pushed_down &&
105-
!rinfo->outerjoin_delayed)
111+
!bms_is_member(rel->relid, rinfo->nullable_relids))
106112
{
107113
/*
108114
* Use the generate_bitmap_or_paths() machinery to estimate the

src/backend/optimizer/plan/initsplan.c

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.149 2009/02/27 22:41:38 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.150 2009/04/16 20:42:16 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -53,7 +53,7 @@ static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
5353
Relids ojscope,
5454
Relids outerjoin_nonnullable);
5555
static bool check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
56-
bool is_pushed_down);
56+
Relids *nullable_relids_p, bool is_pushed_down);
5757
static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause);
5858
static void check_mergejoinable(RestrictInfo *restrictinfo);
5959
static void check_hashjoinable(RestrictInfo *restrictinfo);
@@ -755,6 +755,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
755755
bool pseudoconstant = false;
756756
bool maybe_equivalence;
757757
bool maybe_outer_join;
758+
Relids nullable_relids;
758759
RestrictInfo *restrictinfo;
759760

760761
/*
@@ -861,6 +862,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
861862
Assert(!ojscope);
862863
is_pushed_down = true;
863864
outerjoin_delayed = false;
865+
nullable_relids = NULL;
864866
/* Don't feed it back for more deductions */
865867
maybe_equivalence = false;
866868
maybe_outer_join = false;
@@ -882,7 +884,10 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
882884
maybe_outer_join = true;
883885

884886
/* Check to see if must be delayed by lower outer join */
885-
outerjoin_delayed = check_outerjoin_delay(root, &relids, false);
887+
outerjoin_delayed = check_outerjoin_delay(root,
888+
&relids,
889+
&nullable_relids,
890+
false);
886891

887892
/*
888893
* Now force the qual to be evaluated exactly at the level of joining
@@ -907,7 +912,10 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
907912
is_pushed_down = true;
908913

909914
/* Check to see if must be delayed by lower outer join */
910-
outerjoin_delayed = check_outerjoin_delay(root, &relids, true);
915+
outerjoin_delayed = check_outerjoin_delay(root,
916+
&relids,
917+
&nullable_relids,
918+
true);
911919

912920
if (outerjoin_delayed)
913921
{
@@ -957,7 +965,8 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
957965
is_pushed_down,
958966
outerjoin_delayed,
959967
pseudoconstant,
960-
relids);
968+
relids,
969+
nullable_relids);
961970

962971
/*
963972
* If it's a join clause (either naturally, or because delayed by
@@ -1064,7 +1073,9 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
10641073
* If the qual must be delayed, add relids to *relids_p to reflect the lowest
10651074
* safe level for evaluating the qual, and return TRUE. Any extra delay for
10661075
* higher-level joins is reflected by setting delay_upper_joins to TRUE in
1067-
* SpecialJoinInfo structs.
1076+
* SpecialJoinInfo structs. We also compute nullable_relids, the set of
1077+
* referenced relids that are nullable by lower outer joins (note that this
1078+
* can be nonempty even for a non-delayed qual).
10681079
*
10691080
* For an is_pushed_down qual, we can evaluate the qual as soon as (1) we have
10701081
* all the rels it mentions, and (2) we are at or above any outer joins that
@@ -1087,8 +1098,8 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
10871098
* mentioning only C cannot be applied below the join to A.
10881099
*
10891100
* For a non-pushed-down qual, this isn't going to determine where we place the
1090-
* qual, but we need to determine outerjoin_delayed anyway for possible use
1091-
* in reconsider_outer_join_clauses().
1101+
* qual, but we need to determine outerjoin_delayed and nullable_relids anyway
1102+
* for use later in the planning process.
10921103
*
10931104
* Lastly, a pushed-down qual that references the nullable side of any current
10941105
* join_info_list member and has to be evaluated above that OJ (because its
@@ -1104,13 +1115,26 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
11041115
* two OJs to commute.)
11051116
*/
11061117
static bool
1107-
check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
1118+
check_outerjoin_delay(PlannerInfo *root,
1119+
Relids *relids_p, /* in/out parameter */
1120+
Relids *nullable_relids_p, /* output parameter */
11081121
bool is_pushed_down)
11091122
{
1110-
Relids relids = *relids_p;
1123+
Relids relids;
1124+
Relids nullable_relids;
11111125
bool outerjoin_delayed;
11121126
bool found_some;
11131127

1128+
/* fast path if no special joins */
1129+
if (root->join_info_list == NIL)
1130+
{
1131+
*nullable_relids_p = NULL;
1132+
return false;
1133+
}
1134+
1135+
/* must copy relids because we need the original value at the end */
1136+
relids = bms_copy(*relids_p);
1137+
nullable_relids = NULL;
11141138
outerjoin_delayed = false;
11151139
do
11161140
{
@@ -1126,18 +1150,23 @@ check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
11261150
(sjinfo->jointype == JOIN_FULL &&
11271151
bms_overlap(relids, sjinfo->min_lefthand)))
11281152
{
1129-
/* yes, so set the result flag */
1130-
outerjoin_delayed = true;
1131-
/* have we included all its rels in relids? */
1153+
/* yes; have we included all its rels in relids? */
11321154
if (!bms_is_subset(sjinfo->min_lefthand, relids) ||
11331155
!bms_is_subset(sjinfo->min_righthand, relids))
11341156
{
11351157
/* no, so add them in */
11361158
relids = bms_add_members(relids, sjinfo->min_lefthand);
11371159
relids = bms_add_members(relids, sjinfo->min_righthand);
1160+
outerjoin_delayed = true;
11381161
/* we'll need another iteration */
11391162
found_some = true;
11401163
}
1164+
/* track all the nullable rels of relevant OJs */
1165+
nullable_relids = bms_add_members(nullable_relids,
1166+
sjinfo->min_righthand);
1167+
if (sjinfo->jointype == JOIN_FULL)
1168+
nullable_relids = bms_add_members(nullable_relids,
1169+
sjinfo->min_lefthand);
11411170
/* set delay_upper_joins if needed */
11421171
if (is_pushed_down && sjinfo->jointype != JOIN_FULL &&
11431172
bms_overlap(relids, sjinfo->min_lefthand))
@@ -1146,7 +1175,13 @@ check_outerjoin_delay(PlannerInfo *root, Relids *relids_p,
11461175
}
11471176
} while (found_some);
11481177

1178+
/* identify just the actually-referenced nullable rels */
1179+
nullable_relids = bms_int_members(nullable_relids, *relids_p);
1180+
1181+
/* replace *relids_p, and return nullable_relids */
1182+
bms_free(*relids_p);
11491183
*relids_p = relids;
1184+
*nullable_relids_p = nullable_relids;
11501185
return outerjoin_delayed;
11511186
}
11521187

@@ -1352,7 +1387,8 @@ build_implied_join_equality(Oid opno,
13521387
true, /* is_pushed_down */
13531388
false, /* outerjoin_delayed */
13541389
false, /* pseudoconstant */
1355-
qualscope);
1390+
qualscope, /* required_relids */
1391+
NULL); /* nullable_relids */
13561392

13571393
/* Set mergejoinability info always, and hashjoinability if enabled */
13581394
check_mergejoinable(restrictinfo);

0 commit comments

Comments
 (0)