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

Commit 59857b4

Browse files
committed
Fix create_unique_plan() so it doesn't generate useless entries in the
output targetlist of the Unique or HashAgg plan. This code was OK when written, but subsequent changes to use "physical tlists" where possible had broken it: given an input subplan that has extra variables added to avoid a projection step, it would copy those extra variables into the upper tlist, which is pointless since a projection has to happen anyway.
1 parent 5e544e4 commit 59857b4

File tree

1 file changed

+45
-28
lines changed

1 file changed

+45
-28
lines changed

src/backend/optimizer/plan/createplan.c

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.193 2005/07/02 23:00:41 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.194 2005/07/15 22:02:51 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -525,34 +525,33 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path)
525525
Plan *plan;
526526
Plan *subplan;
527527
List *uniq_exprs;
528-
int numGroupCols;
529-
AttrNumber *groupColIdx;
530-
int groupColPos;
531528
List *newtlist;
532529
int nextresno;
533530
bool newitems;
531+
int numGroupCols;
532+
AttrNumber *groupColIdx;
533+
int groupColPos;
534534
ListCell *l;
535535

536536
subplan = create_plan(root, best_path->subpath);
537537

538+
/* Done if we don't need to do any actual unique-ifying */
539+
if (best_path->umethod == UNIQUE_PATH_NOOP)
540+
return subplan;
541+
538542
/*----------
539543
* As constructed, the subplan has a "flat" tlist containing just the
540544
* Vars needed here and at upper levels. The values we are supposed
541545
* to unique-ify may be expressions in these variables. We have to
542-
* add any such expressions to the subplan's tlist. We then build
543-
* control information showing which subplan output columns are to be
544-
* examined by the grouping step. (Since we do not remove any
545-
* existing subplan outputs, not all the output columns may be used
546-
* for grouping.)
546+
* add any such expressions to the subplan's tlist.
547547
*
548-
* Note: the reason we don't remove any subplan outputs is that there
549-
* are scenarios where a Var is needed at higher levels even though
550-
* it is not one of the nominal outputs of an IN clause. Consider
551-
* WHERE x IN (SELECT y FROM t1,t2 WHERE y = z)
552-
* Implied equality deduction will generate an "x = z" clause, which may
553-
* get used instead of "x = y" in the upper join step. Therefore the
554-
* sub-select had better deliver both y and z in its targetlist.
555-
* It is sufficient to unique-ify on y, however.
548+
* The subplan may have a "physical" tlist if it is a simple scan plan.
549+
* This should be left as-is if we don't need to add any expressions;
550+
* but if we do have to add expressions, then a projection step will be
551+
* needed at runtime anyway, and so we may as well remove unneeded items.
552+
* Therefore newtlist starts from build_relation_tlist() not just a
553+
* copy of the subplan's tlist; and we don't install it into the subplan
554+
* unless stuff has to be added.
556555
*
557556
* To find the correct list of values to unique-ify, we look in the
558557
* information saved for IN expressions. If this code is ever used in
@@ -574,12 +573,8 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path)
574573
if (l == NULL) /* fell out of loop? */
575574
elog(ERROR, "could not find UniquePath in in_info_list");
576575

577-
/* set up to record positions of unique columns */
578-
numGroupCols = list_length(uniq_exprs);
579-
groupColIdx = (AttrNumber *) palloc(numGroupCols * sizeof(AttrNumber));
580-
groupColPos = 0;
581-
/* not sure if tlist might be shared with other nodes, so copy */
582-
newtlist = copyObject(subplan->targetlist);
576+
/* initialize modified subplan tlist as just the "required" vars */
577+
newtlist = build_relation_tlist(best_path->path.parent);
583578
nextresno = list_length(newtlist) + 1;
584579
newitems = false;
585580

@@ -599,7 +594,6 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path)
599594
nextresno++;
600595
newitems = true;
601596
}
602-
groupColIdx[groupColPos++] = tle->resno;
603597
}
604598

605599
if (newitems)
@@ -614,18 +608,41 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path)
614608
subplan->targetlist = newtlist;
615609
}
616610

617-
/* Done if we don't need to do any actual unique-ifying */
618-
if (best_path->umethod == UNIQUE_PATH_NOOP)
619-
return subplan;
611+
/*
612+
* Build control information showing which subplan output columns are
613+
* to be examined by the grouping step. Unfortunately we can't merge this
614+
* with the previous loop, since we didn't then know which version of the
615+
* subplan tlist we'd end up using.
616+
*/
617+
newtlist = subplan->targetlist;
618+
numGroupCols = list_length(uniq_exprs);
619+
groupColIdx = (AttrNumber *) palloc(numGroupCols * sizeof(AttrNumber));
620+
groupColPos = 0;
621+
622+
foreach(l, uniq_exprs)
623+
{
624+
Node *uniqexpr = lfirst(l);
625+
TargetEntry *tle;
626+
627+
tle = tlist_member(uniqexpr, newtlist);
628+
if (!tle) /* shouldn't happen */
629+
elog(ERROR, "failed to find unique expression in subplan tlist");
630+
groupColIdx[groupColPos++] = tle->resno;
631+
}
620632

621633
if (best_path->umethod == UNIQUE_PATH_HASH)
622634
{
623635
long numGroups;
624636

625637
numGroups = (long) Min(best_path->rows, (double) LONG_MAX);
626638

639+
/*
640+
* Since the Agg node is going to project anyway, we can give it
641+
* the minimum output tlist, without any stuff we might have added
642+
* to the subplan tlist.
643+
*/
627644
plan = (Plan *) make_agg(root,
628-
copyObject(subplan->targetlist),
645+
build_relation_tlist(best_path->path.parent),
629646
NIL,
630647
AGG_HASHED,
631648
numGroupCols,

0 commit comments

Comments
 (0)