Skip to content

Commit a1fbd47

Browse files
vadim4ovadim4o
committed
Fix GroupBy: enable functions over aggregates and GroupBy-ed fields
in target list.
1 parent f4279c4 commit a1fbd47

File tree

3 files changed

+87
-66
lines changed

3 files changed

+87
-66
lines changed

‎src/backend/optimizer/plan/planmain.c

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planmain.c,v 1.3 1997/04/05 06:37:37 vadim Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planmain.c,v 1.4 1997/04/29 04:32:50 vadim Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -359,8 +359,9 @@ make_groupPlan(List **tlist,
359359
List *sort_tlist;
360360
List *sl, *gl;
361361
List *glc = listCopy (groupClause);
362-
List *aggvals = NIL; /* list of vars of aggregates */
363-
int aggvcnt;
362+
List *otles = NIL; /* list of removed non-GroupBy entries */
363+
List *otlvars = NIL; /* list of var in them */
364+
int otlvcnt;
364365
Sort *sortplan;
365366
Group *grpplan;
366367
int numCols;
@@ -375,7 +376,8 @@ make_groupPlan(List **tlist,
375376

376377
/*
377378
* Make template TL for subplan, Sort & Group:
378-
* 1. Take away Aggregates and re-set resno-s accordantly.
379+
* 1. If there are aggregates (tuplePerGroup is true) then take
380+
* away non-GroupBy entries and re-set resno-s accordantly.
379381
* 2. Make grpColIdx
380382
*
381383
* Note: we assume that TLEs in *tlist are ordered in accordance
@@ -390,7 +392,7 @@ make_groupPlan(List **tlist,
390392
{
391393
GroupClause *grpcl = (GroupClause*)lfirst(gl);
392394

393-
if ( grpcl->resdom->resno == te->resdom->resno )
395+
if ( grpcl->entry->resdom->resno == te->resdom->resno )
394396
{
395397

396398
resdom = te->resdom;
@@ -403,15 +405,20 @@ make_groupPlan(List **tlist,
403405
break;
404406
}
405407
}
406-
if ( resdom == NULL ) /* Not GroupBy-ed entry: remove */
407-
{ /* aggregate(s) from Group/Sort TL */
408-
if ( IsA (te->expr, Aggreg) )
409-
{ /* save Aggregate' Vars */
410-
aggvals = nconc (aggvals, pull_var_clause (te->expr));
411-
sort_tlist = lremove (lfirst (sl), sort_tlist);
408+
/*
409+
* Non-GroupBy entry: remove it from Group/Sort TL if there are
410+
* aggregates in query - it will be evaluated by Aggregate plan
411+
*/
412+
if ( resdom == NULL )
413+
{
414+
if ( tuplePerGroup )
415+
{
416+
otlvars = nconc (otlvars, pull_var_clause (te->expr));
417+
otles = lcons (te, otles);
418+
sort_tlist = lremove (te, sort_tlist);
412419
}
413420
else
414-
resdom->resno = last_resno++; /* re-set */
421+
te->resdom->resno = last_resno++;
415422
}
416423
}
417424

@@ -421,12 +428,12 @@ make_groupPlan(List **tlist,
421428
}
422429

423430
/*
424-
* Aggregates were removed from TL - we are to add Vars for them
425-
* to the end of TL if there are no such Vars in TL already.
431+
* If non-GroupBy entries were removed from TL - we are to add Vars for
432+
* them to the end of TL if there are no such Vars in TL already.
426433
*/
427434

428-
aggvcnt = length (aggvals);
429-
foreach (gl, aggvals)
435+
otlvcnt = length (otlvars);
436+
foreach (gl, otlvars)
430437
{
431438
Var *v = (Var*)lfirst (gl);
432439

@@ -437,9 +444,9 @@ make_groupPlan(List **tlist,
437444
last_resno++;
438445
}
439446
else /* already in TL */
440-
aggvcnt--;
447+
otlvcnt--;
441448
}
442-
/* Now aggvcnt is number of Vars added in TL for Aggregates */
449+
/* Now otlvcnt is number of Vars added in TL for non-GroupBy entries */
443450

444451
/* Make TL for subplan: substitute Vars from subplan TL into new TL */
445452
sl = flatten_tlist_vars (sort_tlist, subplan->targetlist);
@@ -489,17 +496,28 @@ make_groupPlan(List **tlist,
489496
grpColIdx, sortplan);
490497

491498
/*
492-
* Make TL for parent: "restore" Aggregates and
493-
* resno-s of others accordantly.
499+
* Make TL for parent: "restore" non-GroupBy entries (if they
500+
* were removed) and set resno-s of others accordantly.
494501
*/
495502
sl = sort_tlist;
496503
sort_tlist = NIL; /* to be new parent TL */
497504
foreach (gl, *tlist)
498505
{
506+
List *temp = NIL;
499507
TargetEntry *te = (TargetEntry *) lfirst (gl);
500-
501-
if ( !IsA (te->expr, Aggreg) ) /* It's "our" TLE - we're to return */
502-
{ /* it from Sort/Group plans */
508+
509+
foreach (temp, otles) /* Is it removed non-GroupBy entry ? */
510+
{
511+
TargetEntry *ote = (TargetEntry *) lfirst (temp);
512+
513+
if ( ote->resdom->resno == te->resdom->resno )
514+
{
515+
otles = lremove (ote, otles);
516+
break;
517+
}
518+
}
519+
if ( temp == NIL ) /* It's "our" TLE - we're to return */
520+
{ /* it from Sort/Group plans */
503521
TargetEntry *my = (TargetEntry *) lfirst (sl); /* get it */
504522

505523
sl = sl->next; /* prepare for the next "our" */
@@ -508,15 +526,16 @@ make_groupPlan(List **tlist,
508526
sort_tlist = lappend (sort_tlist, my);
509527
continue;
510528
}
511-
/* TLE of an aggregate */
529+
/* else - it's TLE of an non-GroupBy entry */
512530
sort_tlist = lappend (sort_tlist, copyObject(te));
513531
}
514532
/*
515-
* Pure aggregates Vars were at the end of Group' TL.
533+
* Pure non-GroupBy entries Vars were at the end of Group' TL.
516534
* They shouldn't appear in parent TL, all others shouldn't
517535
* disappear.
518536
*/
519-
Assert ( aggvcnt == length (sl) );
537+
Assert ( otlvcnt == length (sl) );
538+
Assert ( length (otles) == 0 );
520539

521540
*tlist = sort_tlist;
522541

‎src/backend/parser/analyze.c

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.26 1997/04/27 19:16:44 thomas Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.27 1997/04/29 04:32:26 vadim Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -1552,7 +1552,8 @@ transformGroupClause(ParseState *pstate, List *grouplist, List *targetlist)
15521552
if (restarget == NULL)
15531553
elog(WARN,"The field being grouped by must appear in the target list");
15541554

1555-
grpcl->resdom = resdom = restarget->resdom;
1555+
grpcl->entry = restarget;
1556+
resdom = restarget->resdom;
15561557
grpcl->grpOpoid = oprid(oper("<",
15571558
resdom->restype,
15581559
resdom->restype,false));
@@ -2359,6 +2360,39 @@ contain_agg_clause(Node *clause)
23592360
return FALSE;
23602361
}
23612362

2363+
/*
2364+
* exprIsAggOrGroupCol -
2365+
* returns true if the expression does not contain non-group columns.
2366+
*/
2367+
static bool
2368+
exprIsAggOrGroupCol(Node *expr, List *groupClause)
2369+
{
2370+
List *gl;
2371+
2372+
if ( expr == NULL || IsA (expr, Const) || IsA (expr, Aggreg) )
2373+
return TRUE;
2374+
2375+
foreach (gl, groupClause)
2376+
{
2377+
GroupClause *grpcl = lfirst(gl);
2378+
2379+
if ( equal (expr, grpcl->entry->expr) )
2380+
return TRUE;
2381+
}
2382+
2383+
if ( IsA (expr, Expr) )
2384+
{
2385+
List *temp;
2386+
2387+
foreach (temp, ((Expr*)expr)->args)
2388+
if (!exprIsAggOrGroupCol(lfirst(temp),groupClause))
2389+
return FALSE;
2390+
return TRUE;
2391+
}
2392+
2393+
return FALSE;
2394+
}
2395+
23622396
/*
23632397
* tleIsAggOrGroupCol -
23642398
* returns true if the TargetEntry is Agg or GroupCol.
@@ -2376,9 +2410,9 @@ tleIsAggOrGroupCol(TargetEntry *tle, List *groupClause)
23762410
{
23772411
GroupClause *grpcl = lfirst(gl);
23782412

2379-
if ( tle->resdom->resno == grpcl->resdom->resno )
2413+
if ( tle->resdom->resno == grpcl->entry->resdom->resno )
23802414
{
2381-
if ( IsA (expr, Aggreg) )
2415+
if ( contain_agg_clause ((Node*) expr) )
23822416
elog (WARN, "parser: aggregates not allowed in GROUP BY clause");
23832417
return TRUE;
23842418
}
@@ -2387,39 +2421,8 @@ tleIsAggOrGroupCol(TargetEntry *tle, List *groupClause)
23872421
if ( IsA (expr, Aggreg) )
23882422
return TRUE;
23892423

2390-
return FALSE;
2391-
}
2392-
2393-
#if 0 /* Now GroupBy contains resdom to enable Group By func_results */
2394-
/*
2395-
* exprIsAggOrGroupCol -
2396-
* returns true if the expression does not contain non-group columns.
2397-
*/
2398-
static bool
2399-
exprIsAggOrGroupCol(Node *expr, List *groupClause)
2400-
{
2401-
if (expr==NULL)
2402-
return TRUE;
2403-
else if (IsA(expr,Const))
2404-
return TRUE;
2405-
else if (IsA(expr,Var)) {
2406-
List *gl;
2407-
Var *var = (Var*)expr;
2408-
/*
2409-
* only group columns are legal
2410-
*/
2411-
foreach (gl, groupClause) {
2412-
GroupClause *grpcl = lfirst(gl);
2413-
if ((grpcl->grpAttr->varno == var->varno) &&
2414-
(grpcl->grpAttr->varattno == var->varattno))
2415-
return TRUE;
2416-
}
2417-
return FALSE;
2418-
} else if (IsA(expr,Aggreg))
2419-
/* aggregates can take group column or non-group column as argument,
2420-
no further check necessary. */
2421-
return TRUE;
2422-
else if (IsA(expr,Expr)) {
2424+
if ( IsA (expr, Expr) )
2425+
{
24232426
List *temp;
24242427

24252428
foreach (temp, ((Expr*)expr)->args)
@@ -2430,7 +2433,6 @@ exprIsAggOrGroupCol(Node *expr, List *groupClause)
24302433

24312434
return FALSE;
24322435
}
2433-
#endif
24342436

24352437
/*
24362438
* parseCheckAggregates -

‎src/include/nodes/parsenodes.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*
77
* Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Id: parsenodes.h,v 1.14 1997/04/23 05:58:06 vadim Exp $
9+
* $Id: parsenodes.h,v 1.15 1997/04/29 04:28:59 vadim Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -774,7 +774,7 @@ typedef struct SortClause {
774774
*/
775775
typedef struct GroupClause {
776776
NodeTag type;
777-
Resdom *resdom; /* attributes to group on */
777+
TargetEntry *entry; /* attributes to group on */
778778
Oid grpOpoid; /* the sort operator to use */
779779
} GroupClause;
780780

0 commit comments

Comments
 (0)