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

Commit ecbed6e

Browse files
committed
create_unique_plan() should not discard existing output columns of the
subplan it starts with, as they may be needed at upper join levels. See comments added to code for the non-obvious reason why. Per bug report from Robert Creager.
1 parent d862045 commit ecbed6e

File tree

5 files changed

+95
-38
lines changed

5 files changed

+95
-38
lines changed

src/backend/optimizer/plan/createplan.c

+69-32
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.151 2003/08/04 02:40:00 momjian Exp $
13+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.152 2003/08/07 19:20:22 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -504,52 +504,87 @@ create_unique_plan(Query *root, UniquePath * best_path)
504504
{
505505
Plan *plan;
506506
Plan *subplan;
507-
List *sub_targetlist;
507+
List *uniq_exprs;
508+
int numGroupCols;
509+
AttrNumber *groupColIdx;
510+
int groupColPos;
511+
List *newtlist;
512+
int nextresno;
513+
bool newitems;
508514
List *my_tlist;
509515
List *l;
510516

511517
subplan = create_plan(root, best_path->subpath);
512518

513519
/*
514-
* If the subplan came from an IN subselect (currently always the
515-
* case), we need to instantiate the correct output targetlist for the
516-
* subselect, rather than using the flattened tlist.
520+
* As constructed, the subplan has a "flat" tlist containing just the
521+
* Vars needed here and at upper levels. The values we are supposed
522+
* to unique-ify may be expressions in these variables. We have to
523+
* add any such expressions to the subplan's tlist. We then build
524+
* control information showing which subplan output columns are to be
525+
* examined by the grouping step. (Since we do not remove any existing
526+
* subplan outputs, not all the output columns may be used for grouping.)
527+
*
528+
* Note: the reason we don't remove any subplan outputs is that there
529+
* are scenarios where a Var is needed at higher levels even though it
530+
* is not one of the nominal outputs of an IN clause. Consider
531+
* WHERE x IN (SELECT y FROM t1,t2 WHERE y = z)
532+
* Implied equality deduction will generate an "x = z" clause, which may
533+
* get used instead of "x = y" in the upper join step. Therefore the
534+
* sub-select had better deliver both y and z in its targetlist. It is
535+
* sufficient to unique-ify on y, however.
536+
*
537+
* To find the correct list of values to unique-ify, we look in the
538+
* information saved for IN expressions. If this code is ever used in
539+
* other scenarios, some other way of finding what to unique-ify will
540+
* be needed.
517541
*/
518-
sub_targetlist = NIL;
542+
uniq_exprs = NIL; /* just to keep compiler quiet */
519543
foreach(l, root->in_info_list)
520544
{
521545
InClauseInfo *ininfo = (InClauseInfo *) lfirst(l);
522546

523547
if (bms_equal(ininfo->righthand, best_path->path.parent->relids))
524548
{
525-
sub_targetlist = ininfo->sub_targetlist;
549+
uniq_exprs = ininfo->sub_targetlist;
526550
break;
527551
}
528552
}
529-
530-
if (sub_targetlist)
553+
if (l == NIL) /* fell out of loop? */
554+
elog(ERROR, "could not find UniquePath in in_info_list");
555+
556+
/* set up to record positions of unique columns */
557+
numGroupCols = length(uniq_exprs);
558+
groupColIdx = (AttrNumber *) palloc(numGroupCols * sizeof(AttrNumber));
559+
groupColPos = 0;
560+
/* not sure if tlist might be shared with other nodes, so copy */
561+
newtlist = copyObject(subplan->targetlist);
562+
nextresno = length(newtlist) + 1;
563+
newitems = false;
564+
565+
foreach(l, uniq_exprs)
531566
{
532-
/*
533-
* Transform list of plain Vars into targetlist
534-
*/
535-
List *newtlist = NIL;
536-
int resno = 1;
567+
Node *uniqexpr = lfirst(l);
568+
TargetEntry *tle;
537569

538-
foreach(l, sub_targetlist)
570+
tle = tlistentry_member(uniqexpr, newtlist);
571+
if (!tle)
539572
{
540-
Node *tlexpr = lfirst(l);
541-
TargetEntry *tle;
542-
543-
tle = makeTargetEntry(makeResdom(resno,
544-
exprType(tlexpr),
545-
exprTypmod(tlexpr),
573+
tle = makeTargetEntry(makeResdom(nextresno,
574+
exprType(uniqexpr),
575+
exprTypmod(uniqexpr),
546576
NULL,
547577
false),
548-
(Expr *) tlexpr);
578+
(Expr *) uniqexpr);
549579
newtlist = lappend(newtlist, tle);
550-
resno++;
580+
nextresno++;
581+
newitems = true;
551582
}
583+
groupColIdx[groupColPos++] = tle->resdom->resno;
584+
}
552585

586+
if (newitems)
587+
{
553588
/*
554589
* If the top plan node can't do projections, we need to add a
555590
* Result node to help it along.
@@ -563,21 +598,15 @@ create_unique_plan(Query *root, UniquePath * best_path)
563598
subplan->targetlist = newtlist;
564599
}
565600

601+
/* Copy tlist again to make one we can put sorting labels on */
566602
my_tlist = copyObject(subplan->targetlist);
567603

568604
if (best_path->use_hash)
569605
{
570-
int numGroupCols = length(my_tlist);
571606
long numGroups;
572-
AttrNumber *groupColIdx;
573-
int i;
574607

575608
numGroups = (long) Min(best_path->rows, (double) LONG_MAX);
576609

577-
groupColIdx = (AttrNumber *) palloc(numGroupCols * sizeof(AttrNumber));
578-
for (i = 0; i < numGroupCols; i++)
579-
groupColIdx[i] = i + 1;
580-
581610
plan = (Plan *) make_agg(root,
582611
my_tlist,
583612
NIL,
@@ -590,9 +619,17 @@ create_unique_plan(Query *root, UniquePath * best_path)
590619
}
591620
else
592621
{
593-
List *sortList;
622+
List *sortList = NIL;
594623

595-
sortList = addAllTargetsToSortList(NULL, NIL, my_tlist, false);
624+
for (groupColPos = 0; groupColPos < numGroupCols; groupColPos++)
625+
{
626+
TargetEntry *tle;
627+
628+
tle = nth(groupColIdx[groupColPos] - 1, my_tlist);
629+
Assert(tle->resdom->resno == groupColIdx[groupColPos]);
630+
sortList = addTargetToSortList(NULL, tle, sortList,
631+
my_tlist, NIL, false);
632+
}
596633
plan = (Plan *) make_sort_from_sortclauses(root, my_tlist,
597634
subplan, sortList);
598635
plan = (Plan *) make_unique(my_tlist, plan, sortList);

src/backend/parser/parse_clause.c

+2-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.120 2003/08/04 02:40:01 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.121 2003/08/07 19:20:22 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -59,9 +59,6 @@ static Node *buildMergedJoinVar(ParseState *pstate, JoinType jointype,
5959
Var *l_colvar, Var *r_colvar);
6060
static TargetEntry *findTargetlistEntry(ParseState *pstate, Node *node,
6161
List *tlist, int clause);
62-
static List *addTargetToSortList(ParseState *pstate, TargetEntry *tle,
63-
List *sortlist, List *targetlist,
64-
List *opname, bool resolveUnknown);
6562

6663

6764
/*
@@ -1478,7 +1475,7 @@ addAllTargetsToSortList(ParseState *pstate, List *sortlist,
14781475
*
14791476
* Returns the updated ORDER BY list.
14801477
*/
1481-
static List *
1478+
List *
14821479
addTargetToSortList(ParseState *pstate, TargetEntry *tle,
14831480
List *sortlist, List *targetlist,
14841481
List *opname, bool resolveUnknown)

src/include/parser/parse_clause.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: parse_clause.h,v 1.35 2003/08/04 02:40:14 momjian Exp $
10+
* $Id: parse_clause.h,v 1.36 2003/08/07 19:20:23 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -35,6 +35,9 @@ extern List *transformDistinctClause(ParseState *pstate, List *distinctlist,
3535
extern List *addAllTargetsToSortList(ParseState *pstate,
3636
List *sortlist, List *targetlist,
3737
bool resolveUnknown);
38+
extern List *addTargetToSortList(ParseState *pstate, TargetEntry *tle,
39+
List *sortlist, List *targetlist,
40+
List *opname, bool resolveUnknown);
3841
extern Index assignSortGroupRef(TargetEntry *tle, List *tlist);
3942
extern bool targetIsInSortList(TargetEntry *tle, List *sortList);
4043

src/test/regress/expected/join.out

+12
Original file line numberDiff line numberDiff line change
@@ -2127,6 +2127,18 @@ on (x1 = xx1) where (xx2 is not null);
21272127
4 | 44 | 4 | | 4 | 44
21282128
(3 rows)
21292129

2130+
--
2131+
-- regression test: check for bug with propagation of implied equality
2132+
-- to outside an IN
2133+
--
2134+
select count(*) from tenk1 a where unique1 in
2135+
(select unique1 from tenk1 b join tenk1 c using (unique1)
2136+
where b.unique2 = 42);
2137+
count
2138+
-------
2139+
1
2140+
(1 row)
2141+
21302142
--
21312143
-- Clean up
21322144
--

src/test/regress/sql/join.sql

+8
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,14 @@ on (x1 = xx1) where (y2 is not null);
330330
select * from (x left join y on (x1 = y1)) left join x xx(xx1,xx2)
331331
on (x1 = xx1) where (xx2 is not null);
332332

333+
--
334+
-- regression test: check for bug with propagation of implied equality
335+
-- to outside an IN
336+
--
337+
select count(*) from tenk1 a where unique1 in
338+
(select unique1 from tenk1 b join tenk1 c using (unique1)
339+
where b.unique2 = 42);
340+
333341

334342
--
335343
-- Clean up

0 commit comments

Comments
 (0)