Skip to content

Commit 1bdf124

Browse files
tglsfdctglsfdc
committed
Restore the former RestrictInfo field valid_everywhere (but invert the flag
sense and rename to "outerjoin_delayed" to more clearly reflect what it means). I had decided that it was redundant in 8.1, but the folly of this is exposed by a bug report from Sebastian Böck. The place where it's needed is to prevent orindxpath.c from cherry-picking arms of an outer-join OR clause to form a relation restriction that isn't actually legal to push down to the relation scan level. There may be some legal cases that this forbids optimizing, but we'd need much closer analysis to determine it.
1 parent e93fb88 commit 1bdf124

File tree

9 files changed

+72
-26
lines changed

9 files changed

+72
-26
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.316 2005/10/15 02:49:18 momjian Exp $
18+
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.317 2005/11/14 23:54:12 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -1249,6 +1249,7 @@ _copyRestrictInfo(RestrictInfo *from)
12491249

12501250
COPY_NODE_FIELD(clause);
12511251
COPY_SCALAR_FIELD(is_pushed_down);
1252+
COPY_SCALAR_FIELD(outerjoin_delayed);
12521253
COPY_SCALAR_FIELD(can_join);
12531254
COPY_BITMAPSET_FIELD(clause_relids);
12541255
COPY_BITMAPSET_FIELD(required_relids);

‎src/backend/nodes/equalfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* Portions Copyright (c) 1994, Regents of the University of California
1919
*
2020
* IDENTIFICATION
21-
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.253 2005/10/15 02:49:18 momjian Exp $
21+
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.254 2005/11/14 23:54:15 tgl Exp $
2222
*
2323
*-------------------------------------------------------------------------
2424
*/
@@ -602,6 +602,7 @@ _equalRestrictInfo(RestrictInfo *a, RestrictInfo *b)
602602
{
603603
COMPARE_NODE_FIELD(clause);
604604
COMPARE_SCALAR_FIELD(is_pushed_down);
605+
COMPARE_SCALAR_FIELD(outerjoin_delayed);
605606
COMPARE_BITMAPSET_FIELD(required_relids);
606607

607608
/*

‎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.261 2005/10/15 02:49:18 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.262 2005/11/14 23:54:15 tgl Exp $
1212
*
1313
* NOTES
1414
* Every node type that can appear in stored rules' parsetrees *must*
@@ -1241,6 +1241,7 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
12411241
/* NB: this isn't a complete set of fields */
12421242
WRITE_NODE_FIELD(clause);
12431243
WRITE_BOOL_FIELD(is_pushed_down);
1244+
WRITE_BOOL_FIELD(outerjoin_delayed);
12441245
WRITE_BOOL_FIELD(can_join);
12451246
WRITE_BITMAPSET_FIELD(clause_relids);
12461247
WRITE_BITMAPSET_FIELD(required_relids);

‎src/backend/optimizer/path/indxpath.c

Lines changed: 7 additions & 6 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.191 2005/10/15 02:49:19 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.192 2005/11/14 23:54:17 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -1917,6 +1917,7 @@ expand_indexqual_conditions(IndexOptInfo *index, List *clausegroups)
19171917
resultquals = lappend(resultquals,
19181918
make_restrictinfo(boolqual,
19191919
true,
1920+
false,
19201921
NULL));
19211922
continue;
19221923
}
@@ -2166,7 +2167,7 @@ prefix_quals(Node *leftop, Oid opclass,
21662167
elog(ERROR, "no = operator for opclass %u", opclass);
21672168
expr = make_opclause(oproid, BOOLOID, false,
21682169
(Expr *) leftop, (Expr *) prefix_const);
2169-
result = list_make1(make_restrictinfo(expr, true, NULL));
2170+
result = list_make1(make_restrictinfo(expr, true, false, NULL));
21702171
return result;
21712172
}
21722173

@@ -2181,7 +2182,7 @@ prefix_quals(Node *leftop, Oid opclass,
21812182
elog(ERROR, "no >= operator for opclass %u", opclass);
21822183
expr = make_opclause(oproid, BOOLOID, false,
21832184
(Expr *) leftop, (Expr *) prefix_const);
2184-
result = list_make1(make_restrictinfo(expr, true, NULL));
2185+
result = list_make1(make_restrictinfo(expr, true, false, NULL));
21852186

21862187
/*-------
21872188
* If we can create a string larger than the prefix, we can say
@@ -2197,7 +2198,7 @@ prefix_quals(Node *leftop, Oid opclass,
21972198
elog(ERROR, "no < operator for opclass %u", opclass);
21982199
expr = make_opclause(oproid, BOOLOID, false,
21992200
(Expr *) leftop, (Expr *) greaterstr);
2200-
result = lappend(result, make_restrictinfo(expr, true, NULL));
2201+
result = lappend(result, make_restrictinfo(expr, true, false, NULL));
22012202
}
22022203

22032204
return result;
@@ -2268,7 +2269,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opclass, Datum rightop)
22682269
(Expr *) leftop,
22692270
(Expr *) makeConst(datatype, -1, opr1right,
22702271
false, false));
2271-
result = list_make1(make_restrictinfo(expr, true, NULL));
2272+
result = list_make1(make_restrictinfo(expr, true, false, NULL));
22722273

22732274
/* create clause "key <= network_scan_last( rightop )" */
22742275

@@ -2283,7 +2284,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opclass, Datum rightop)
22832284
(Expr *) leftop,
22842285
(Expr *) makeConst(datatype, -1, opr2right,
22852286
false, false));
2286-
result = lappend(result, make_restrictinfo(expr, true, NULL));
2287+
result = lappend(result, make_restrictinfo(expr, true, false, NULL));
22872288

22882289
return result;
22892290
}

‎src/backend/optimizer/path/orindxpath.c

Lines changed: 9 additions & 3 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.75 2005/10/15 02:49:20 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.76 2005/11/14 23:54:18 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -90,13 +90,19 @@ create_or_index_quals(PlannerInfo *root, RelOptInfo *rel)
9090
ListCell *i;
9191

9292
/*
93-
* Find potentially interesting OR joinclauses.
93+
* Find potentially interesting OR joinclauses. Note we must ignore any
94+
* joinclauses that are marked outerjoin_delayed, because they cannot
95+
* be pushed down to the per-relation level due to outer-join rules.
96+
* (XXX in some cases it might be possible to allow this, but it would
97+
* require substantially more bookkeeping about where the clause came
98+
* from.)
9499
*/
95100
foreach(i, rel->joininfo)
96101
{
97102
RestrictInfo *rinfo = (RestrictInfo *) lfirst(i);
98103

99-
if (restriction_is_or_clause(rinfo))
104+
if (restriction_is_or_clause(rinfo) &&
105+
!rinfo->outerjoin_delayed)
100106
{
101107
/*
102108
* Use the generate_bitmap_or_paths() machinery to estimate the

‎src/backend/optimizer/plan/initsplan.c

Lines changed: 9 additions & 1 deletion
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.110 2005/10/15 02:49:20 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.111 2005/11/14 23:54:18 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -409,6 +409,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
409409
Relids qualscope)
410410
{
411411
Relids relids;
412+
bool outerjoin_delayed;
412413
bool maybe_equijoin;
413414
bool maybe_outer_join;
414415
RestrictInfo *restrictinfo;
@@ -451,6 +452,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
451452
*/
452453
Assert(bms_equal(relids, qualscope));
453454
/* Needn't feed it back for more deductions */
455+
outerjoin_delayed = false;
454456
maybe_equijoin = false;
455457
maybe_outer_join = false;
456458
}
@@ -470,6 +472,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
470472
* except for not setting maybe_equijoin (see below).
471473
*/
472474
relids = qualscope;
475+
outerjoin_delayed = true;
473476

474477
/*
475478
* We can't use such a clause to deduce equijoin (the left and right
@@ -499,13 +502,17 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
499502
Relids tmprelids;
500503
int relno;
501504

505+
outerjoin_delayed = false;
502506
tmprelids = bms_copy(relids);
503507
while ((relno = bms_first_member(tmprelids)) >= 0)
504508
{
505509
RelOptInfo *rel = find_base_rel(root, relno);
506510

507511
if (rel->outerjoinset != NULL)
512+
{
508513
addrelids = bms_add_members(addrelids, rel->outerjoinset);
514+
outerjoin_delayed = true;
515+
}
509516
}
510517
bms_free(tmprelids);
511518

@@ -555,6 +562,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
555562
*/
556563
restrictinfo = make_restrictinfo((Expr *) clause,
557564
is_pushed_down,
565+
outerjoin_delayed,
558566
relids);
559567

560568
/*

‎src/backend/optimizer/util/restrictinfo.c

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.41 2005/10/15 02:49:21 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.42 2005/11/14 23:54:22 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -25,9 +25,11 @@
2525
static RestrictInfo *make_restrictinfo_internal(Expr *clause,
2626
Expr *orclause,
2727
bool is_pushed_down,
28+
bool outerjoin_delayed,
2829
Relids required_relids);
2930
static Expr *make_sub_restrictinfos(Expr *clause,
30-
bool is_pushed_down);
31+
bool is_pushed_down,
32+
bool outerjoin_delayed);
3133
static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
3234
RestrictInfo *rinfo,
3335
List *reference_list,
@@ -39,28 +41,34 @@ static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
3941
*
4042
* Build a RestrictInfo node containing the given subexpression.
4143
*
42-
* The is_pushed_down flag must be supplied by the caller.
43-
* required_relids can be NULL, in which case it defaults to the
44+
* The is_pushed_down and outerjoin_delayed flags must be supplied by the
45+
* caller. required_relids can be NULL, in which case it defaults to the
4446
* actual clause contents (i.e., clause_relids).
4547
*
4648
* We initialize fields that depend only on the given subexpression, leaving
4749
* others that depend on context (or may never be needed at all) to be filled
4850
* later.
4951
*/
5052
RestrictInfo *
51-
make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids)
53+
make_restrictinfo(Expr *clause,
54+
bool is_pushed_down,
55+
bool outerjoin_delayed,
56+
Relids required_relids)
5257
{
5358
/*
5459
* If it's an OR clause, build a modified copy with RestrictInfos inserted
5560
* above each subclause of the top-level AND/OR structure.
5661
*/
5762
if (or_clause((Node *) clause))
58-
return (RestrictInfo *) make_sub_restrictinfos(clause, is_pushed_down);
63+
return (RestrictInfo *) make_sub_restrictinfos(clause,
64+
is_pushed_down,
65+
outerjoin_delayed);
5966

6067
/* Shouldn't be an AND clause, else AND/OR flattening messed up */
6168
Assert(!and_clause((Node *) clause));
6269

63-
return make_restrictinfo_internal(clause, NULL, is_pushed_down,
70+
return make_restrictinfo_internal(clause, NULL,
71+
is_pushed_down, outerjoin_delayed,
6472
required_relids);
6573
}
6674

@@ -74,6 +82,9 @@ make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids)
7482
* The result is a List (effectively, implicit-AND representation) of
7583
* RestrictInfos.
7684
*
85+
* The caller must pass is_pushed_down, but we assume outerjoin_delayed
86+
* is false (no such qual should ever get into a bitmapqual).
87+
*
7788
* If include_predicates is true, we add any partial index predicates to
7889
* the explicit index quals. When this is not true, we return a condition
7990
* that might be weaker than the actual scan represents.
@@ -169,6 +180,7 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
169180
list_make1(make_restrictinfo_internal(make_orclause(withoutris),
170181
make_orclause(withris),
171182
is_pushed_down,
183+
false,
172184
NULL));
173185
}
174186
}
@@ -193,6 +205,7 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
193205
result = lappend(result,
194206
make_restrictinfo(pred,
195207
is_pushed_down,
208+
false,
196209
NULL));
197210
}
198211
}
@@ -213,13 +226,15 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
213226
*/
214227
static RestrictInfo *
215228
make_restrictinfo_internal(Expr *clause, Expr *orclause,
216-
bool is_pushed_down, Relids required_relids)
229+
bool is_pushed_down, bool outerjoin_delayed,
230+
Relids required_relids)
217231
{
218232
RestrictInfo *restrictinfo = makeNode(RestrictInfo);
219233

220234
restrictinfo->clause = clause;
221235
restrictinfo->orclause = orclause;
222236
restrictinfo->is_pushed_down = is_pushed_down;
237+
restrictinfo->outerjoin_delayed = outerjoin_delayed;
223238
restrictinfo->can_join = false; /* may get set below */
224239

225240
/*
@@ -299,7 +314,8 @@ make_restrictinfo_internal(Expr *clause, Expr *orclause,
299314
* simple clauses are valid RestrictInfos.
300315
*/
301316
static Expr *
302-
make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
317+
make_sub_restrictinfos(Expr *clause,
318+
bool is_pushed_down, bool outerjoin_delayed)
303319
{
304320
if (or_clause((Node *) clause))
305321
{
@@ -309,10 +325,12 @@ make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
309325
foreach(temp, ((BoolExpr *) clause)->args)
310326
orlist = lappend(orlist,
311327
make_sub_restrictinfos(lfirst(temp),
312-
is_pushed_down));
328+
is_pushed_down,
329+
outerjoin_delayed));
313330
return (Expr *) make_restrictinfo_internal(clause,
314331
make_orclause(orlist),
315332
is_pushed_down,
333+
outerjoin_delayed,
316334
NULL);
317335
}
318336
else if (and_clause((Node *) clause))
@@ -323,13 +341,15 @@ make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
323341
foreach(temp, ((BoolExpr *) clause)->args)
324342
andlist = lappend(andlist,
325343
make_sub_restrictinfos(lfirst(temp),
326-
is_pushed_down));
344+
is_pushed_down,
345+
outerjoin_delayed));
327346
return make_andclause(andlist);
328347
}
329348
else
330349
return (Expr *) make_restrictinfo_internal(clause,
331350
NULL,
332351
is_pushed_down,
352+
outerjoin_delayed,
333353
NULL);
334354
}
335355

‎src/include/nodes/relation.h

Lines changed: 8 additions & 1 deletion
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/nodes/relation.h,v 1.119 2005/10/15 02:49:45 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.120 2005/11/14 23:54:23 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -714,6 +714,11 @@ typedef struct HashPath
714714
* joined, will also have is_pushed_down set because it will get attached to
715715
* some lower joinrel.
716716
*
717+
* When application of a qual must be delayed by outer join, we also mark it
718+
* with outerjoin_delayed = true. This isn't redundant with required_relids
719+
* because that might equal clause_relids whether or not it's an outer-join
720+
* clause.
721+
*
717722
* In general, the referenced clause might be arbitrarily complex. The
718723
* kinds of clauses we can handle as indexscan quals, mergejoin clauses,
719724
* or hashjoin clauses are fairly limited --- the code for each kind of
@@ -740,6 +745,8 @@ typedef struct RestrictInfo
740745

741746
bool is_pushed_down; /* TRUE if clause was pushed down in level */
742747

748+
bool outerjoin_delayed; /* TRUE if delayed by outer join */
749+
743750
/*
744751
* This flag is set true if the clause looks potentially useful as a merge
745752
* or hash join clause, that is if it is a binary opclause with

‎src/include/optimizer/restrictinfo.h

Lines changed: 2 additions & 1 deletion
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/restrictinfo.h,v 1.34 2005/10/15 02:49:45 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.35 2005/11/14 23:54:23 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -19,6 +19,7 @@
1919

2020
extern RestrictInfo *make_restrictinfo(Expr *clause,
2121
bool is_pushed_down,
22+
bool outerjoin_delayed,
2223
Relids required_relids);
2324
extern List *make_restrictinfo_from_bitmapqual(Path *bitmapqual,
2425
bool is_pushed_down,

0 commit comments

Comments
 (0)