Skip to content

Commit 595ed2a

Browse files
tglsfdctglsfdc
committed
Make the behavior of HAVING without GROUP BY conform to the SQL spec.
Formerly, if such a clause contained no aggregate functions we mistakenly treated it as equivalent to WHERE. Per spec it must cause the query to be treated as a grouped query of a single group, the same as appearance of aggregate functions would do. Also, the HAVING filter must execute after aggregate function computation even if it itself contains no aggregate functions.
1 parent 609e32b commit 595ed2a

File tree

20 files changed

+384
-237
lines changed

20 files changed

+384
-237
lines changed

‎doc/src/sgml/query.sgml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/query.sgml,v 1.43 2005/01/22 22:56:36 momjian Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/query.sgml,v 1.44 2005/03/10 23:21:20 tgl Exp $
33
-->
44

55
<chapter id="tutorial-sql">
@@ -783,8 +783,9 @@ SELECT city, max(temp_lo)
783783
will be inputs to the aggregates. On the other hand, the
784784
<literal>HAVING</literal> clause always contains aggregate functions.
785785
(Strictly speaking, you are allowed to write a <literal>HAVING</literal>
786-
clause that doesn't use aggregates, but it's wasteful. The same condition
787-
could be used more efficiently at the <literal>WHERE</literal> stage.)
786+
clause that doesn't use aggregates, but it's seldom useful. The same
787+
condition could be used more efficiently at the <literal>WHERE</literal>
788+
stage.)
788789
</para>
789790

790791
<para>

‎doc/src/sgml/ref/select.sgml

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/select.sgml,v 1.81 2005/01/22 23:22:19 momjian Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/select.sgml,v 1.82 2005/03/10 23:21:20 tgl Exp $
33
PostgreSQL documentation
44
-->
55

@@ -452,6 +452,17 @@ HAVING <replaceable class="parameter">condition</replaceable>
452452
unambiguously reference a grouping column, unless the reference
453453
appears within an aggregate function.
454454
</para>
455+
456+
<para>
457+
The presence of <literal>HAVING</literal> turns a query into a grouped
458+
query even if there is no <literal>GROUP BY</> clause. This is the
459+
same as what happens when the query contains aggregate functions but
460+
no <literal>GROUP BY</> clause. All the selected rows are considered to
461+
form a single group, and the <command>SELECT</command> list and
462+
<literal>HAVING</literal> clause can only reference table columns from
463+
within aggregate functions. Such a query will emit a single row if the
464+
<literal>HAVING</literal> condition is true, zero rows if it is not true.
465+
</para>
455466
</refsect2>
456467

457468
<refsect2 id="sql-select-list">

‎src/backend/executor/nodeGroup.c

Lines changed: 75 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* locate group boundaries.
1616
*
1717
* IDENTIFICATION
18-
* $PostgreSQL: pgsql/src/backend/executor/nodeGroup.c,v 1.59 2004/12/31 21:59:45 pgsql Exp $
18+
* $PostgreSQL: pgsql/src/backend/executor/nodeGroup.c,v 1.60 2005/03/10 23:21:21 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -35,23 +35,19 @@
3535
TupleTableSlot *
3636
ExecGroup(GroupState *node)
3737
{
38-
EState *estate;
3938
ExprContext *econtext;
4039
TupleDesc tupdesc;
4140
int numCols;
4241
AttrNumber *grpColIdx;
43-
HeapTuple outerTuple = NULL;
42+
HeapTuple outerTuple;
4443
HeapTuple firsttuple;
4544
TupleTableSlot *outerslot;
46-
ProjectionInfo *projInfo;
47-
TupleTableSlot *resultSlot;
4845

4946
/*
5047
* get state info from node
5148
*/
5249
if (node->grp_done)
5350
return NULL;
54-
estate = node->ss.ps.state;
5551
econtext = node->ss.ps.ps_ExprContext;
5652
tupdesc = ExecGetScanType(&node->ss);
5753
numCols = ((Group *) node->ss.ps.plan)->numCols;
@@ -62,67 +58,101 @@ ExecGroup(GroupState *node)
6258
* reset the per-tuple memory context once per input tuple.
6359
*/
6460

65-
/* If we don't already have first tuple of group, fetch it */
66-
/* this should occur on the first call only */
61+
/*
62+
* If first time through, acquire first input tuple and determine
63+
* whether to return it or not.
64+
*/
6765
firsttuple = node->grp_firstTuple;
6866
if (firsttuple == NULL)
6967
{
7068
outerslot = ExecProcNode(outerPlanState(node));
7169
if (TupIsNull(outerslot))
7270
{
71+
/* empty input, so return nothing */
7372
node->grp_done = TRUE;
7473
return NULL;
7574
}
76-
node->grp_firstTuple = firsttuple =
77-
heap_copytuple(outerslot->val);
75+
node->grp_firstTuple = firsttuple = heap_copytuple(outerslot->val);
76+
/* Set up tuple as input for qual test and projection */
77+
ExecStoreTuple(firsttuple,
78+
node->ss.ss_ScanTupleSlot,
79+
InvalidBuffer,
80+
false);
81+
econtext->ecxt_scantuple = node->ss.ss_ScanTupleSlot;
82+
/*
83+
* Check the qual (HAVING clause); if the group does not match,
84+
* ignore it and fall into scan loop.
85+
*/
86+
if (ExecQual(node->ss.ps.qual, econtext, false))
87+
{
88+
/*
89+
* Form and return a projection tuple using the first input
90+
* tuple.
91+
*/
92+
return ExecProject(node->ss.ps.ps_ProjInfo, NULL);
93+
}
7894
}
7995

8096
/*
81-
* Scan over all tuples that belong to this group
97+
* This loop iterates once per input tuple group. At the head of the
98+
* loop, we have finished processing the first tuple of the group and
99+
* now need to scan over all the other group members.
82100
*/
83101
for (;;)
84102
{
85-
outerslot = ExecProcNode(outerPlanState(node));
86-
if (TupIsNull(outerslot))
103+
/*
104+
* Scan over all remaining tuples that belong to this group
105+
*/
106+
for (;;)
87107
{
88-
node->grp_done = TRUE;
89-
outerTuple = NULL;
90-
break;
91-
}
92-
outerTuple = outerslot->val;
108+
outerslot = ExecProcNode(outerPlanState(node));
109+
if (TupIsNull(outerslot))
110+
{
111+
/* no more groups, so we're done */
112+
node->grp_done = TRUE;
113+
return NULL;
114+
}
115+
outerTuple = outerslot->val;
93116

117+
/*
118+
* Compare with first tuple and see if this tuple is of the same
119+
* group. If so, ignore it and keep scanning.
120+
*/
121+
if (!execTuplesMatch(firsttuple, outerTuple,
122+
tupdesc,
123+
numCols, grpColIdx,
124+
node->eqfunctions,
125+
econtext->ecxt_per_tuple_memory))
126+
break;
127+
}
94128
/*
95-
* Compare with first tuple and see if this tuple is of the same
96-
* group.
129+
* We have the first tuple of the next input group. See if we
130+
* want to return it.
97131
*/
98-
if (!execTuplesMatch(firsttuple, outerTuple,
99-
tupdesc,
100-
numCols, grpColIdx,
101-
node->eqfunctions,
102-
econtext->ecxt_per_tuple_memory))
103-
break;
104-
}
105-
106-
/*
107-
* form a projection tuple based on the (copied) first tuple of the
108-
* group, and store it in the result tuple slot.
109-
*/
110-
ExecStoreTuple(firsttuple,
111-
node->ss.ss_ScanTupleSlot,
112-
InvalidBuffer,
113-
false);
114-
econtext->ecxt_scantuple = node->ss.ss_ScanTupleSlot;
115-
projInfo = node->ss.ps.ps_ProjInfo;
116-
resultSlot = ExecProject(projInfo, NULL);
117-
118-
/* save first tuple of next group, if we are not done yet */
119-
if (!node->grp_done)
120-
{
121132
heap_freetuple(firsttuple);
122-
node->grp_firstTuple = heap_copytuple(outerTuple);
133+
node->grp_firstTuple = firsttuple = heap_copytuple(outerTuple);
134+
/* Set up tuple as input for qual test and projection */
135+
ExecStoreTuple(firsttuple,
136+
node->ss.ss_ScanTupleSlot,
137+
InvalidBuffer,
138+
false);
139+
econtext->ecxt_scantuple = node->ss.ss_ScanTupleSlot;
140+
/*
141+
* Check the qual (HAVING clause); if the group does not match,
142+
* ignore it and loop back to scan the rest of the group.
143+
*/
144+
if (ExecQual(node->ss.ps.qual, econtext, false))
145+
{
146+
/*
147+
* Form and return a projection tuple using the first input
148+
* tuple.
149+
*/
150+
return ExecProject(node->ss.ps.ps_ProjInfo, NULL);
151+
}
123152
}
124153

125-
return resultSlot;
154+
/* NOTREACHED */
155+
return NULL;
126156
}
127157

128158
/* -----------------

‎src/backend/nodes/copyfuncs.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* Portions Copyright (c) 1994, Regents of the University of California
1616
*
1717
* IDENTIFICATION
18-
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.296 2005/01/27 03:17:45 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.297 2005/03/10 23:21:21 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -1544,14 +1544,15 @@ _copyQuery(Query *from)
15441544
COPY_NODE_FIELD(resultRelations);
15451545
COPY_NODE_FIELD(in_info_list);
15461546
COPY_SCALAR_FIELD(hasJoinRTEs);
1547+
COPY_SCALAR_FIELD(hasHavingQual);
15471548

15481549
/*
15491550
* We do not copy the other planner internal fields: base_rel_list,
15501551
* other_rel_list, join_rel_list, equi_key_list, query_pathkeys. That
15511552
* would get us into copying RelOptInfo/Path trees, which we don't
1552-
* want to do. It is necessary to copy in_info_list and hasJoinRTEs
1553-
* for the benefit of inheritance_planner(), which may try to copy a
1554-
* Query in which these are already set.
1553+
* want to do. It is necessary to copy in_info_list, hasJoinRTEs,
1554+
* and hasHavingQual for the benefit of inheritance_planner(), which
1555+
* may try to copy a Query in which these are already set.
15551556
*/
15561557

15571558
return newnode;

‎src/backend/nodes/equalfuncs.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* Portions Copyright (c) 1994, Regents of the University of California
1919
*
2020
* IDENTIFICATION
21-
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.235 2005/01/27 03:17:45 tgl Exp $
21+
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.236 2005/03/10 23:21:21 tgl Exp $
2222
*
2323
*-------------------------------------------------------------------------
2424
*/
@@ -661,14 +661,10 @@ _equalQuery(Query *a, Query *b)
661661
COMPARE_NODE_FIELD(limitCount);
662662
COMPARE_NODE_FIELD(setOperations);
663663
COMPARE_NODE_FIELD(resultRelations);
664-
COMPARE_NODE_FIELD(in_info_list);
665-
COMPARE_SCALAR_FIELD(hasJoinRTEs);
666664

667665
/*
668-
* We do not check the other planner internal fields: base_rel_list,
669-
* other_rel_list, join_rel_list, equi_key_list, query_pathkeys. They
670-
* might not be set yet, and in any case they should be derivable from
671-
* the other fields.
666+
* We do not check the planner-internal fields. They might not be set
667+
* yet, and in any case they should be derivable from the other fields.
672668
*/
673669
return true;
674670
}

‎src/backend/optimizer/path/allpaths.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.123 2004/12/31 22:00:00 pgsql Exp $
11+
* $PostgreSQL: pgsql/src/backend/optimizer/path/allpaths.c,v 1.124 2005/03/10 23:21:21 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -334,13 +334,10 @@ set_subquery_pathlist(Query *root, RelOptInfo *rel,
334334

335335
/*
336336
* If there are any restriction clauses that have been attached to the
337-
* subquery relation, consider pushing them down to become HAVING
338-
* quals of the subquery itself. (Not WHERE clauses, since they may
339-
* refer to subquery outputs that are aggregate results. But
340-
* planner.c will transfer them into the subquery's WHERE if they do
341-
* not.) This transformation is useful because it may allow us to
342-
* generate a better plan for the subquery than evaluating all the
343-
* subquery output rows and then filtering them.
337+
* subquery relation, consider pushing them down to become WHERE or
338+
* HAVING quals of the subquery itself. This transformation is useful
339+
* because it may allow us to generate a better plan for the subquery
340+
* than evaluating all the subquery output rows and then filtering them.
344341
*
345342
* There are several cases where we cannot push down clauses.
346343
* Restrictions involving the subquery are checked by
@@ -795,8 +792,17 @@ subquery_push_qual(Query *subquery, List *rtable, Index rti, Node *qual)
795792
qual = ResolveNew(qual, rti, 0, rtable,
796793
subquery->targetList,
797794
CMD_SELECT, 0);
798-
subquery->havingQual = make_and_qual(subquery->havingQual,
799-
qual);
795+
796+
/*
797+
* Now attach the qual to the proper place: normally WHERE, but
798+
* if the subquery uses grouping or aggregation, put it in HAVING
799+
* (since the qual really refers to the group-result rows).
800+
*/
801+
if (subquery->hasAggs || subquery->groupClause || subquery->havingQual)
802+
subquery->havingQual = make_and_qual(subquery->havingQual, qual);
803+
else
804+
subquery->jointree->quals =
805+
make_and_qual(subquery->jointree->quals, qual);
800806

801807
/*
802808
* We need not change the subquery's hasAggs or hasSublinks flags,

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

Lines changed: 12 additions & 3 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.175 2004/12/31 22:00:08 pgsql Exp $
13+
* $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.176 2005/03/10 23:21:22 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -2158,6 +2158,7 @@ make_agg(Query *root, List *tlist, List *qual,
21582158
Group *
21592159
make_group(Query *root,
21602160
List *tlist,
2161+
List *qual,
21612162
int numGroupCols,
21622163
AttrNumber *grpColIdx,
21632164
double numGroups,
@@ -2184,7 +2185,8 @@ make_group(Query *root,
21842185
plan->plan_rows = numGroups;
21852186

21862187
/*
2187-
* We also need to account for the cost of evaluation of the tlist.
2188+
* We also need to account for the cost of evaluation of the qual (ie,
2189+
* the HAVING clause) and the tlist.
21882190
*
21892191
* XXX this double-counts the cost of evaluation of any expressions used
21902192
* for grouping, since in reality those will have been evaluated at a
@@ -2194,12 +2196,19 @@ make_group(Query *root,
21942196
* See notes in grouping_planner about why this routine and make_agg are
21952197
* the only ones in this file that worry about tlist eval cost.
21962198
*/
2199+
if (qual)
2200+
{
2201+
cost_qual_eval(&qual_cost, qual);
2202+
plan->startup_cost += qual_cost.startup;
2203+
plan->total_cost += qual_cost.startup;
2204+
plan->total_cost += qual_cost.per_tuple * plan->plan_rows;
2205+
}
21972206
cost_qual_eval(&qual_cost, tlist);
21982207
plan->startup_cost += qual_cost.startup;
21992208
plan->total_cost += qual_cost.startup;
22002209
plan->total_cost += qual_cost.per_tuple * plan->plan_rows;
22012210

2202-
plan->qual = NIL;
2211+
plan->qual = qual;
22032212
plan->targetlist = tlist;
22042213
plan->lefttree = lefttree;
22052214
plan->righttree = NULL;

0 commit comments

Comments
 (0)