From: Tom Lane Date: Mon, 14 Nov 2005 23:54:23 +0000 (+0000) Subject: Restore the former RestrictInfo field valid_everywhere (but invert the flag X-Git-Tag: REL8_2_BETA1~1933 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1bdf124b94af3c24d3c3083c820804274df8262b;p=postgresql 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. --- diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 4a90b10b27..a5efda5a30 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 9baa79dd93..dbebe8d4e8 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 19306b3e53..16acd5d721 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 1790cc5266..0033b98d57 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 be5a0c3434..10b12890da 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 dd8fc4fa2d..fd1ddedbfd 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 d277cac735..96c207c935 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 01aa96d717..15d5647282 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 0715df59d6..4ca5c87bae 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,