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

Commit 9d91db4

Browse files
committed
Repair bug reported by Wickstrom: backend would crash if WHERE clause
contained a sub-SELECT nested within an AND/OR tree that cnfify() thought it should rearrange. Same physical sub-SELECT node could end up linked into multiple places in resulting expression tree. This is harmless for most node types, but not for SubLink. Repair bug by making physical copies of subexpressions that get logically duplicated by cnfify(). Also, tweak the heuristic that decides whether it's a good idea to do cnfify() --- we don't really want that to happen when it would cause multiple copies of a subselect to be generated, I think.
1 parent 2e67eca commit 9d91db4

File tree

2 files changed

+51
-11
lines changed

2 files changed

+51
-11
lines changed

src/backend/optimizer/plan/subselect.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.35 2000/04/12 17:15:22 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.36 2000/04/14 00:19:16 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -136,6 +136,13 @@ make_subplan(SubLink *slink)
136136

137137
PlannerQueryLevel++; /* we become child */
138138

139+
/* Check to see if this node was already processed; if so we have
140+
* trouble. Someday should change tree representation so that we can
141+
* cope with multiple links to the same subquery, but for now...
142+
*/
143+
if (subquery == NULL)
144+
elog(ERROR, "make_subplan: invalid expression structure (subquery already processed?)");
145+
139146
/*
140147
* For an EXISTS subplan, tell lower-level planner to expect that only
141148
* the first tuple will be retrieved. For ALL and ANY subplans, we
@@ -194,7 +201,7 @@ make_subplan(SubLink *slink)
194201
node->plan_id = PlannerPlanId++;
195202
node->rtable = subquery->rtable;
196203
node->sublink = slink;
197-
slink->subselect = NULL; /* cool ?! */
204+
slink->subselect = NULL; /* cool ?! see error check above! */
198205

199206
/* make parParam list of params coming from current query level */
200207
foreach(lst, plan->extParam)

src/backend/optimizer/prep/prepqual.c

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepqual.c,v 1.24 2000/04/12 17:15:23 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepqual.c,v 1.25 2000/04/14 00:19:17 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -656,15 +656,26 @@ or_normalize(List *orlist)
656656
foreach(temp, distributable->args)
657657
{
658658
Expr *andclause = lfirst(temp);
659+
List *neworlist;
660+
661+
/*
662+
* We are going to insert the orlist into multiple places in the
663+
* result expression. For most expression types, it'd be OK to
664+
* just have multiple links to the same subtree, but this fails
665+
* badly for SubLinks (and perhaps other cases?). For safety,
666+
* we make a distinct copy for each place the orlist is inserted.
667+
*/
668+
if (lnext(temp) == NIL)
669+
neworlist = orlist; /* can use original tree at the end */
670+
else
671+
neworlist = copyObject(orlist);
659672

660673
/*
661674
* pull_ors is needed here in case andclause has a top-level OR.
662675
* Then we recursively apply or_normalize, since there might be an
663-
* AND subclause in the resulting OR-list. Note: we rely on
664-
* pull_ors to build a fresh list, and not damage the given
665-
* orlist.
676+
* AND subclause in the resulting OR-list.
666677
*/
667-
andclause = or_normalize(pull_ors(lcons(andclause, orlist)));
678+
andclause = or_normalize(pull_ors(lcons(andclause, neworlist)));
668679
andclauses = lappend(andclauses, andclause);
669680
}
670681

@@ -773,15 +784,26 @@ and_normalize(List *andlist)
773784
foreach(temp, distributable->args)
774785
{
775786
Expr *orclause = lfirst(temp);
787+
List *newandlist;
788+
789+
/*
790+
* We are going to insert the andlist into multiple places in the
791+
* result expression. For most expression types, it'd be OK to
792+
* just have multiple links to the same subtree, but this fails
793+
* badly for SubLinks (and perhaps other cases?). For safety,
794+
* we make a distinct copy for each place the andlist is inserted.
795+
*/
796+
if (lnext(temp) == NIL)
797+
newandlist = andlist; /* can use original tree at the end */
798+
else
799+
newandlist = copyObject(andlist);
776800

777801
/*
778802
* pull_ands is needed here in case orclause has a top-level AND.
779803
* Then we recursively apply and_normalize, since there might be
780-
* an OR subclause in the resulting AND-list. Note: we rely on
781-
* pull_ands to build a fresh list, and not damage the given
782-
* andlist.
804+
* an OR subclause in the resulting AND-list.
783805
*/
784-
orclause = and_normalize(pull_ands(lcons(orclause, andlist)));
806+
orclause = and_normalize(pull_ands(lcons(orclause, newandlist)));
785807
orclauses = lappend(orclauses, orclause);
786808
}
787809

@@ -932,6 +954,17 @@ count_bool_nodes(Expr *qual,
932954
count_bool_nodes(get_notclausearg(qual),
933955
nodes, cnfnodes, dnfnodes);
934956
}
957+
else if (contain_subplans((Node *) qual))
958+
{
959+
/* charge extra for subexpressions containing sub-SELECTs,
960+
* to discourage us from rearranging them in a way that might
961+
* generate N copies of a subselect rather than one. The magic
962+
* constant here interacts with the "4x maximum growth" heuristic
963+
* in canonicalize_qual().
964+
*/
965+
*nodes = 1.0;
966+
*cnfnodes = *dnfnodes = 25.0;
967+
}
935968
else
936969
{
937970
/* anything else counts 1 for my purposes */

0 commit comments

Comments
 (0)