From 1bdf124b94af3c24d3c3083c820804274df8262b Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 14 Nov 2005 23:54:23 +0000
Subject: [PATCH] =?UTF-8?q?Restore=20the=20former=20RestrictInfo=20field?=
=?UTF-8?q?=20valid=5Feverywhere=20(but=20invert=20the=20flag=20sense=20an?=
=?UTF-8?q?d=20rename=20to=20"outerjoin=5Fdelayed"=20to=20more=20clearly?=
=?UTF-8?q?=20reflect=20what=20it=20means).=20=20I=20had=20decided=20that?=
=?UTF-8?q?=20it=20was=20redundant=20in=208.1,=20but=20the=20folly=20of=20?=
=?UTF-8?q?this=20is=20exposed=20by=20a=20bug=20report=20from=20Sebastian?=
=?UTF-8?q?=20B=C3=B6ck.=20=20The=20place=20where=20it's=20needed=20is=20t?=
=?UTF-8?q?o=20prevent=20orindxpath.c=20from=20cherry-picking=20arms=20of?=
=?UTF-8?q?=20an=20outer-join=20OR=20clause=20to=20form=20a=20relation=20r?=
=?UTF-8?q?estriction=20that=20isn't=20actually=20legal=20to=20push=20down?=
=?UTF-8?q?=20to=20the=20relation=20scan=20level.=20=20There=20may=20be=20?=
=?UTF-8?q?some=20legal=20cases=20that=20this=20forbids=20optimizing,=20bu?=
=?UTF-8?q?t=20we'd=20need=20much=20closer=20analysis=20to=20determine=20i?=
=?UTF-8?q?t.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
src/backend/nodes/copyfuncs.c | 3 +-
src/backend/nodes/equalfuncs.c | 3 +-
src/backend/nodes/outfuncs.c | 3 +-
src/backend/optimizer/path/indxpath.c | 13 +++----
src/backend/optimizer/path/orindxpath.c | 12 +++++--
src/backend/optimizer/plan/initsplan.c | 10 +++++-
src/backend/optimizer/util/restrictinfo.c | 42 +++++++++++++++++------
src/include/nodes/relation.h | 9 ++++-
src/include/optimizer/restrictinfo.h | 3 +-
9 files changed, 72 insertions(+), 26 deletions(-)
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 4a90b10b277..a5efda5a30b 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -15,7 +15,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.316 2005/10/15 02:49:18 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.317 2005/11/14 23:54:12 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1249,6 +1249,7 @@ _copyRestrictInfo(RestrictInfo *from)
COPY_NODE_FIELD(clause);
COPY_SCALAR_FIELD(is_pushed_down);
+ COPY_SCALAR_FIELD(outerjoin_delayed);
COPY_SCALAR_FIELD(can_join);
COPY_BITMAPSET_FIELD(clause_relids);
COPY_BITMAPSET_FIELD(required_relids);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 9baa79dd935..dbebe8d4e87 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -18,7 +18,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.253 2005/10/15 02:49:18 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.254 2005/11/14 23:54:15 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -602,6 +602,7 @@ _equalRestrictInfo(RestrictInfo *a, RestrictInfo *b)
{
COMPARE_NODE_FIELD(clause);
COMPARE_SCALAR_FIELD(is_pushed_down);
+ COMPARE_SCALAR_FIELD(outerjoin_delayed);
COMPARE_BITMAPSET_FIELD(required_relids);
/*
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 19306b3e53d..16acd5d7214 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.261 2005/10/15 02:49:18 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.262 2005/11/14 23:54:15 tgl Exp $
*
* NOTES
* Every node type that can appear in stored rules' parsetrees *must*
@@ -1241,6 +1241,7 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
/* NB: this isn't a complete set of fields */
WRITE_NODE_FIELD(clause);
WRITE_BOOL_FIELD(is_pushed_down);
+ WRITE_BOOL_FIELD(outerjoin_delayed);
WRITE_BOOL_FIELD(can_join);
WRITE_BITMAPSET_FIELD(clause_relids);
WRITE_BITMAPSET_FIELD(required_relids);
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 1790cc5266b..0033b98d57a 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -9,7 +9,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.191 2005/10/15 02:49:19 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.192 2005/11/14 23:54:17 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1917,6 +1917,7 @@ expand_indexqual_conditions(IndexOptInfo *index, List *clausegroups)
resultquals = lappend(resultquals,
make_restrictinfo(boolqual,
true,
+ false,
NULL));
continue;
}
@@ -2166,7 +2167,7 @@ prefix_quals(Node *leftop, Oid opclass,
elog(ERROR, "no = operator for opclass %u", opclass);
expr = make_opclause(oproid, BOOLOID, false,
(Expr *) leftop, (Expr *) prefix_const);
- result = list_make1(make_restrictinfo(expr, true, NULL));
+ result = list_make1(make_restrictinfo(expr, true, false, NULL));
return result;
}
@@ -2181,7 +2182,7 @@ prefix_quals(Node *leftop, Oid opclass,
elog(ERROR, "no >= operator for opclass %u", opclass);
expr = make_opclause(oproid, BOOLOID, false,
(Expr *) leftop, (Expr *) prefix_const);
- result = list_make1(make_restrictinfo(expr, true, NULL));
+ result = list_make1(make_restrictinfo(expr, true, false, NULL));
/*-------
* If we can create a string larger than the prefix, we can say
@@ -2197,7 +2198,7 @@ prefix_quals(Node *leftop, Oid opclass,
elog(ERROR, "no < operator for opclass %u", opclass);
expr = make_opclause(oproid, BOOLOID, false,
(Expr *) leftop, (Expr *) greaterstr);
- result = lappend(result, make_restrictinfo(expr, true, NULL));
+ result = lappend(result, make_restrictinfo(expr, true, false, NULL));
}
return result;
@@ -2268,7 +2269,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opclass, Datum rightop)
(Expr *) leftop,
(Expr *) makeConst(datatype, -1, opr1right,
false, false));
- result = list_make1(make_restrictinfo(expr, true, NULL));
+ result = list_make1(make_restrictinfo(expr, true, false, NULL));
/* create clause "key <= network_scan_last( rightop )" */
@@ -2283,7 +2284,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opclass, Datum rightop)
(Expr *) leftop,
(Expr *) makeConst(datatype, -1, opr2right,
false, false));
- result = lappend(result, make_restrictinfo(expr, true, NULL));
+ result = lappend(result, make_restrictinfo(expr, true, false, NULL));
return result;
}
diff --git a/src/backend/optimizer/path/orindxpath.c b/src/backend/optimizer/path/orindxpath.c
index be5a0c3434f..10b12890dae 100644
--- a/src/backend/optimizer/path/orindxpath.c
+++ b/src/backend/optimizer/path/orindxpath.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.75 2005/10/15 02:49:20 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.76 2005/11/14 23:54:18 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -90,13 +90,19 @@ create_or_index_quals(PlannerInfo *root, RelOptInfo *rel)
ListCell *i;
/*
- * Find potentially interesting OR joinclauses.
+ * Find potentially interesting OR joinclauses. Note we must ignore any
+ * joinclauses that are marked outerjoin_delayed, because they cannot
+ * be pushed down to the per-relation level due to outer-join rules.
+ * (XXX in some cases it might be possible to allow this, but it would
+ * require substantially more bookkeeping about where the clause came
+ * from.)
*/
foreach(i, rel->joininfo)
{
RestrictInfo *rinfo = (RestrictInfo *) lfirst(i);
- if (restriction_is_or_clause(rinfo))
+ if (restriction_is_or_clause(rinfo) &&
+ !rinfo->outerjoin_delayed)
{
/*
* Use the generate_bitmap_or_paths() machinery to estimate the
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index dd8fc4fa2d7..fd1ddedbfd2 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.110 2005/10/15 02:49:20 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.111 2005/11/14 23:54:18 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -409,6 +409,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
Relids qualscope)
{
Relids relids;
+ bool outerjoin_delayed;
bool maybe_equijoin;
bool maybe_outer_join;
RestrictInfo *restrictinfo;
@@ -451,6 +452,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
*/
Assert(bms_equal(relids, qualscope));
/* Needn't feed it back for more deductions */
+ outerjoin_delayed = false;
maybe_equijoin = false;
maybe_outer_join = false;
}
@@ -470,6 +472,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
* except for not setting maybe_equijoin (see below).
*/
relids = qualscope;
+ outerjoin_delayed = true;
/*
* 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,
Relids tmprelids;
int relno;
+ outerjoin_delayed = false;
tmprelids = bms_copy(relids);
while ((relno = bms_first_member(tmprelids)) >= 0)
{
RelOptInfo *rel = find_base_rel(root, relno);
if (rel->outerjoinset != NULL)
+ {
addrelids = bms_add_members(addrelids, rel->outerjoinset);
+ outerjoin_delayed = true;
+ }
}
bms_free(tmprelids);
@@ -555,6 +562,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
*/
restrictinfo = make_restrictinfo((Expr *) clause,
is_pushed_down,
+ outerjoin_delayed,
relids);
/*
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index d277cac7351..96c207c9350 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.41 2005/10/15 02:49:21 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.42 2005/11/14 23:54:22 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -25,9 +25,11 @@
static RestrictInfo *make_restrictinfo_internal(Expr *clause,
Expr *orclause,
bool is_pushed_down,
+ bool outerjoin_delayed,
Relids required_relids);
static Expr *make_sub_restrictinfos(Expr *clause,
- bool is_pushed_down);
+ bool is_pushed_down,
+ bool outerjoin_delayed);
static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
RestrictInfo *rinfo,
List *reference_list,
@@ -39,8 +41,8 @@ static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
*
* Build a RestrictInfo node containing the given subexpression.
*
- * The is_pushed_down flag must be supplied by the caller.
- * required_relids can be NULL, in which case it defaults to the
+ * The is_pushed_down and outerjoin_delayed flags must be supplied by the
+ * caller. required_relids can be NULL, in which case it defaults to the
* actual clause contents (i.e., clause_relids).
*
* We initialize fields that depend only on the given subexpression, leaving
@@ -48,19 +50,25 @@ static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
* later.
*/
RestrictInfo *
-make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids)
+make_restrictinfo(Expr *clause,
+ bool is_pushed_down,
+ bool outerjoin_delayed,
+ Relids required_relids)
{
/*
* If it's an OR clause, build a modified copy with RestrictInfos inserted
* above each subclause of the top-level AND/OR structure.
*/
if (or_clause((Node *) clause))
- return (RestrictInfo *) make_sub_restrictinfos(clause, is_pushed_down);
+ return (RestrictInfo *) make_sub_restrictinfos(clause,
+ is_pushed_down,
+ outerjoin_delayed);
/* Shouldn't be an AND clause, else AND/OR flattening messed up */
Assert(!and_clause((Node *) clause));
- return make_restrictinfo_internal(clause, NULL, is_pushed_down,
+ return make_restrictinfo_internal(clause, NULL,
+ is_pushed_down, outerjoin_delayed,
required_relids);
}
@@ -74,6 +82,9 @@ make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids)
* The result is a List (effectively, implicit-AND representation) of
* RestrictInfos.
*
+ * The caller must pass is_pushed_down, but we assume outerjoin_delayed
+ * is false (no such qual should ever get into a bitmapqual).
+ *
* If include_predicates is true, we add any partial index predicates to
* the explicit index quals. When this is not true, we return a condition
* that might be weaker than the actual scan represents.
@@ -169,6 +180,7 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
list_make1(make_restrictinfo_internal(make_orclause(withoutris),
make_orclause(withris),
is_pushed_down,
+ false,
NULL));
}
}
@@ -193,6 +205,7 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
result = lappend(result,
make_restrictinfo(pred,
is_pushed_down,
+ false,
NULL));
}
}
@@ -213,13 +226,15 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
*/
static RestrictInfo *
make_restrictinfo_internal(Expr *clause, Expr *orclause,
- bool is_pushed_down, Relids required_relids)
+ bool is_pushed_down, bool outerjoin_delayed,
+ Relids required_relids)
{
RestrictInfo *restrictinfo = makeNode(RestrictInfo);
restrictinfo->clause = clause;
restrictinfo->orclause = orclause;
restrictinfo->is_pushed_down = is_pushed_down;
+ restrictinfo->outerjoin_delayed = outerjoin_delayed;
restrictinfo->can_join = false; /* may get set below */
/*
@@ -299,7 +314,8 @@ make_restrictinfo_internal(Expr *clause, Expr *orclause,
* simple clauses are valid RestrictInfos.
*/
static Expr *
-make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
+make_sub_restrictinfos(Expr *clause,
+ bool is_pushed_down, bool outerjoin_delayed)
{
if (or_clause((Node *) clause))
{
@@ -309,10 +325,12 @@ make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
foreach(temp, ((BoolExpr *) clause)->args)
orlist = lappend(orlist,
make_sub_restrictinfos(lfirst(temp),
- is_pushed_down));
+ is_pushed_down,
+ outerjoin_delayed));
return (Expr *) make_restrictinfo_internal(clause,
make_orclause(orlist),
is_pushed_down,
+ outerjoin_delayed,
NULL);
}
else if (and_clause((Node *) clause))
@@ -323,13 +341,15 @@ make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
foreach(temp, ((BoolExpr *) clause)->args)
andlist = lappend(andlist,
make_sub_restrictinfos(lfirst(temp),
- is_pushed_down));
+ is_pushed_down,
+ outerjoin_delayed));
return make_andclause(andlist);
}
else
return (Expr *) make_restrictinfo_internal(clause,
NULL,
is_pushed_down,
+ outerjoin_delayed,
NULL);
}
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 01aa96d7171..15d5647282c 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.119 2005/10/15 02:49:45 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.120 2005/11/14 23:54:23 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -714,6 +714,11 @@ typedef struct HashPath
* joined, will also have is_pushed_down set because it will get attached to
* some lower joinrel.
*
+ * When application of a qual must be delayed by outer join, we also mark it
+ * with outerjoin_delayed = true. This isn't redundant with required_relids
+ * because that might equal clause_relids whether or not it's an outer-join
+ * clause.
+ *
* In general, the referenced clause might be arbitrarily complex. The
* kinds of clauses we can handle as indexscan quals, mergejoin clauses,
* or hashjoin clauses are fairly limited --- the code for each kind of
@@ -740,6 +745,8 @@ typedef struct RestrictInfo
bool is_pushed_down; /* TRUE if clause was pushed down in level */
+ bool outerjoin_delayed; /* TRUE if delayed by outer join */
+
/*
* This flag is set true if the clause looks potentially useful as a merge
* or hash join clause, that is if it is a binary opclause with
diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h
index 0715df59d68..4ca5c87bae8 100644
--- a/src/include/optimizer/restrictinfo.h
+++ b/src/include/optimizer/restrictinfo.h
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.34 2005/10/15 02:49:45 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.35 2005/11/14 23:54:23 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -19,6 +19,7 @@
extern RestrictInfo *make_restrictinfo(Expr *clause,
bool is_pushed_down,
+ bool outerjoin_delayed,
Relids required_relids);
extern List *make_restrictinfo_from_bitmapqual(Path *bitmapqual,
bool is_pushed_down,
--
GitLab