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

Commit 2e1254e

Browse files
committed
Repair planning bug introduced in 7.4: outer-join ON clauses that referenced
only the inner-side relation would be considered as potential equijoin clauses, which is wrong because the condition doesn't necessarily hold above the point of the outer join. Per test case from Kevin Grittner (bug#1916).
1 parent 4ff2032 commit 2e1254e

File tree

3 files changed

+76
-25
lines changed

3 files changed

+76
-25
lines changed

src/backend/optimizer/plan/initsplan.c

+71-21
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.108 2005/07/02 23:00:41 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.109 2005/09/28 21:17:02 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -38,7 +38,8 @@ static void mark_baserels_for_outer_join(PlannerInfo *root, Relids rels,
3838
Relids outerrels);
3939
static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
4040
bool is_pushed_down,
41-
bool isdeduced,
41+
bool is_deduced,
42+
bool below_outer_join,
4243
Relids outerjoin_nonnullable,
4344
Relids qualscope);
4445
static void add_vars_to_targetlist(PlannerInfo *root, List *vars,
@@ -174,6 +175,10 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
174175
* with outerjoinset information, to aid in proper positioning of qual
175176
* clauses that appear above outer joins.
176177
*
178+
* jtnode is the jointree node currently being examined. below_outer_join
179+
* is TRUE if this node is within the nullable side of a higher-level outer
180+
* join.
181+
*
177182
* NOTE: when dealing with inner joins, it is appropriate to let a qual clause
178183
* be evaluated at the lowest level where all the variables it mentions are
179184
* available. However, we cannot push a qual down into the nullable side(s)
@@ -189,7 +194,8 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
189194
* internal convenience; no outside callers pay attention to the result.
190195
*/
191196
Relids
192-
distribute_quals_to_rels(PlannerInfo *root, Node *jtnode)
197+
distribute_quals_to_rels(PlannerInfo *root, Node *jtnode,
198+
bool below_outer_join)
193199
{
194200
Relids result = NULL;
195201

@@ -214,7 +220,8 @@ distribute_quals_to_rels(PlannerInfo *root, Node *jtnode)
214220
{
215221
result = bms_add_members(result,
216222
distribute_quals_to_rels(root,
217-
lfirst(l)));
223+
lfirst(l),
224+
below_outer_join));
218225
}
219226

220227
/*
@@ -223,7 +230,8 @@ distribute_quals_to_rels(PlannerInfo *root, Node *jtnode)
223230
*/
224231
foreach(l, (List *) f->quals)
225232
distribute_qual_to_rels(root, (Node *) lfirst(l),
226-
true, false, NULL, result);
233+
true, false, below_outer_join,
234+
NULL, result);
227235
}
228236
else if (IsA(jtnode, JoinExpr))
229237
{
@@ -247,27 +255,47 @@ distribute_quals_to_rels(PlannerInfo *root, Node *jtnode)
247255
* rels from being pushed down below this level. (It's okay for
248256
* upper quals to be pushed down to the outer side, however.)
249257
*/
250-
leftids = distribute_quals_to_rels(root, j->larg);
251-
rightids = distribute_quals_to_rels(root, j->rarg);
252-
253-
result = bms_union(leftids, rightids);
254-
255-
nonnullable_rels = nullable_rels = NULL;
256258
switch (j->jointype)
257259
{
258260
case JOIN_INNER:
261+
leftids = distribute_quals_to_rels(root, j->larg,
262+
below_outer_join);
263+
rightids = distribute_quals_to_rels(root, j->rarg,
264+
below_outer_join);
265+
266+
result = bms_union(leftids, rightids);
259267
/* Inner join adds no restrictions for quals */
268+
nonnullable_rels = NULL;
269+
nullable_rels = NULL;
260270
break;
261271
case JOIN_LEFT:
272+
leftids = distribute_quals_to_rels(root, j->larg,
273+
below_outer_join);
274+
rightids = distribute_quals_to_rels(root, j->rarg,
275+
true);
276+
277+
result = bms_union(leftids, rightids);
262278
nonnullable_rels = leftids;
263279
nullable_rels = rightids;
264280
break;
265281
case JOIN_FULL:
282+
leftids = distribute_quals_to_rels(root, j->larg,
283+
true);
284+
rightids = distribute_quals_to_rels(root, j->rarg,
285+
true);
286+
287+
result = bms_union(leftids, rightids);
266288
/* each side is both outer and inner */
267289
nonnullable_rels = result;
268290
nullable_rels = result;
269291
break;
270292
case JOIN_RIGHT:
293+
leftids = distribute_quals_to_rels(root, j->larg,
294+
true);
295+
rightids = distribute_quals_to_rels(root, j->rarg,
296+
below_outer_join);
297+
298+
result = bms_union(leftids, rightids);
271299
nonnullable_rels = rightids;
272300
nullable_rels = leftids;
273301
break;
@@ -280,16 +308,20 @@ distribute_quals_to_rels(PlannerInfo *root, Node *jtnode)
280308
ereport(ERROR,
281309
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
282310
errmsg("UNION JOIN is not implemented")));
311+
nonnullable_rels = NULL; /* keep compiler quiet */
312+
nullable_rels = NULL;
283313
break;
284314
default:
285315
elog(ERROR, "unrecognized join type: %d",
286316
(int) j->jointype);
317+
nonnullable_rels = NULL; /* keep compiler quiet */
318+
nullable_rels = NULL;
287319
break;
288320
}
289321

290322
foreach(qual, (List *) j->quals)
291323
distribute_qual_to_rels(root, (Node *) lfirst(qual),
292-
false, false,
324+
false, false, below_outer_join,
293325
nonnullable_rels, result);
294326

295327
if (nullable_rels != NULL)
@@ -357,7 +389,9 @@ mark_baserels_for_outer_join(PlannerInfo *root, Relids rels, Relids outerrels)
357389
* 'clause': the qual clause to be distributed
358390
* 'is_pushed_down': if TRUE, force the clause to be marked 'is_pushed_down'
359391
* (this indicates the clause came from a FromExpr, not a JoinExpr)
360-
* 'isdeduced': TRUE if the qual came from implied-equality deduction
392+
* 'is_deduced': TRUE if the qual came from implied-equality deduction
393+
* 'below_outer_join': TRUE if the qual is from a JOIN/ON that is below the
394+
* nullable side of a higher-level outer join.
361395
* 'outerjoin_nonnullable': NULL if not an outer-join qual, else the set of
362396
* baserels appearing on the outer (nonnullable) side of the join
363397
* 'qualscope': set of baserels the qual's syntactic scope covers
@@ -369,7 +403,8 @@ mark_baserels_for_outer_join(PlannerInfo *root, Relids rels, Relids outerrels)
369403
static void
370404
distribute_qual_to_rels(PlannerInfo *root, Node *clause,
371405
bool is_pushed_down,
372-
bool isdeduced,
406+
bool is_deduced,
407+
bool below_outer_join,
373408
Relids outerjoin_nonnullable,
374409
Relids qualscope)
375410
{
@@ -406,7 +441,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
406441
* Check to see if clause application must be delayed by outer-join
407442
* considerations.
408443
*/
409-
if (isdeduced)
444+
if (is_deduced)
410445
{
411446
/*
412447
* If the qual came from implied-equality deduction, we always
@@ -432,14 +467,18 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
432467
*
433468
* Note: an outer-join qual that mentions only nullable-side rels can
434469
* be pushed down into the nullable side without changing the join
435-
* result, so we treat it the same as an ordinary inner-join qual.
470+
* result, so we treat it the same as an ordinary inner-join qual,
471+
* except for not setting maybe_equijoin (see below).
436472
*/
437473
relids = qualscope;
438474
/*
439475
* We can't use such a clause to deduce equijoin (the left and
440476
* right sides might be unequal above the join because one of
441477
* them has gone to NULL) ... but we might be able to use it
442-
* for more limited purposes.
478+
* for more limited purposes. Note: for the current uses of
479+
* deductions from an outer-join clause, it seems safe to make
480+
* the deductions even when the clause is below a higher-level
481+
* outer join; so we do not check below_outer_join here.
443482
*/
444483
maybe_equijoin = false;
445484
maybe_outer_join = true;
@@ -473,8 +512,19 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
473512

474513
if (bms_is_subset(addrelids, relids))
475514
{
476-
/* Qual is not affected by any outer-join restriction */
477-
maybe_equijoin = true;
515+
/*
516+
* Qual is not delayed by any lower outer-join restriction.
517+
* If it is not itself below or within an outer join, we
518+
* can consider it "valid everywhere", so consider feeding
519+
* it to the equijoin machinery. (If it is within an outer
520+
* join, we can't consider it "valid everywhere": once the
521+
* contained variables have gone to NULL, we'd be asserting
522+
* things like NULL = NULL, which is not true.)
523+
*/
524+
if (!below_outer_join && outerjoin_nonnullable == NULL)
525+
maybe_equijoin = true;
526+
else
527+
maybe_equijoin = false;
478528
}
479529
else
480530
{
@@ -543,7 +593,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
543593
* redundancy will be detected when the join clause is moved
544594
* into a join rel's restriction list.)
545595
*/
546-
if (!isdeduced ||
596+
if (!is_deduced ||
547597
!qual_is_redundant(root, restrictinfo,
548598
rel->baserestrictinfo))
549599
{
@@ -810,7 +860,7 @@ process_implied_equality(PlannerInfo *root,
810860
* taken for an original JOIN/ON clause.
811861
*/
812862
distribute_qual_to_rels(root, (Node *) clause,
813-
true, true, NULL, relids);
863+
true, true, false, NULL, relids);
814864
}
815865

816866
/*

src/backend/optimizer/plan/planmain.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.87 2005/08/27 22:13:43 tgl Exp $
17+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/planmain.c,v 1.88 2005/09/28 21:17:02 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -155,7 +155,7 @@ query_planner(PlannerInfo *root, List *tlist, double tuple_fraction,
155155
*/
156156
build_base_rel_tlists(root, tlist);
157157

158-
(void) distribute_quals_to_rels(root, (Node *) parse->jointree);
158+
(void) distribute_quals_to_rels(root, (Node *) parse->jointree, false);
159159

160160
/*
161161
* Use the completed lists of equijoined keys to deduce any implied

src/include/optimizer/planmain.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/optimizer/planmain.h,v 1.88 2005/08/27 22:13:44 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/optimizer/planmain.h,v 1.89 2005/09/28 21:17:02 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -67,7 +67,8 @@ extern bool is_projection_capable_plan(Plan *plan);
6767
*/
6868
extern void add_base_rels_to_query(PlannerInfo *root, Node *jtnode);
6969
extern void build_base_rel_tlists(PlannerInfo *root, List *final_tlist);
70-
extern Relids distribute_quals_to_rels(PlannerInfo *root, Node *jtnode);
70+
extern Relids distribute_quals_to_rels(PlannerInfo *root, Node *jtnode,
71+
bool below_outer_join);
7172
extern void process_implied_equality(PlannerInfo *root,
7273
Node *item1, Node *item2,
7374
Oid sortop1, Oid sortop2,

0 commit comments

Comments
 (0)