]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Revert "Add GROUP BY ALL".
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 17 Jul 2026 20:13:57 +0000 (16:13 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 17 Jul 2026 20:14:07 +0000 (16:14 -0400)
This reverts commit ef38a4d9756db9ae1d20f40aa39f3cf76059b81a which
implemented the GROUP BY ALL syntax, as well as 2ce745836 which
made some comment improvements therein.

A postcommit review discovered that GROUP BY ALL missed our special
handling of entries that also appear in an ORDER BY in the query.
This caused the query to return wrong results when ORDER BY
specifies non-default equality semantics.  While this should be
fixable with some refactoring, doing it cleanly seems like too
much code churn for late beta.  We'll revert and try again in v20.

The reverted commit also included some additional comment wordsmithing
and docs cleanup, which are retained as they weren't connected to the
reverted feature.

catversion bump needed due to change in struct Query.

Reported-by: Chao Li <li.evan.chao@gmail.com>
Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/5243308F-8E5C-45AA-828C-FAD96C4F34DA@gmail.com
Backpatch-through: 19

12 files changed:
doc/src/sgml/queries.sgml
doc/src/sgml/ref/select.sgml
doc/src/sgml/ref/select_into.sgml
src/backend/parser/analyze.c
src/backend/parser/gram.y
src/backend/parser/parse_clause.c
src/backend/utils/adt/ruleutils.c
src/include/catalog/catversion.h
src/include/nodes/parsenodes.h
src/include/parser/parse_clause.h
src/test/regress/expected/aggregates.out
src/test/regress/sql/aggregates.sql

index b3cfbc93582b702718eaa5b5636f3f516cf2bcce..3d729f983b54c1227e5f895769e707d1eaa49b39 100644 (file)
@@ -1159,31 +1159,6 @@ SELECT product_id, p.name, (sum(s.units) * p.price) AS sales
     expressions cannot contain aggregate functions or window functions).
    </para>
 
-   <para>
-    PostgreSQL also supports the syntax <literal>GROUP BY ALL</literal>,
-    which is equivalent to explicitly writing all select-list entries that
-    do not contain either an aggregate function referring to the same query level or a window function.
-    This can greatly simplify ad-hoc exploration of data.
-    As an example, these queries are equivalent:
-<screen>
-<prompt>=&gt;</prompt> <userinput>SELECT a, b, a + b, sum(c) FROM test1 GROUP BY ALL;</userinput>
- a | b | ?column? | sum
----+---+----------+----
- 1 | 4 |        5 |  9
- 2 | 5 |        7 | 12
- 3 | 6 |        9 | 15
-(3 rows)
-
-<prompt>=&gt;</prompt> <userinput>SELECT a, b, a + b, sum(c) FROM test1 GROUP BY a, b, a + b;</userinput>
- a | b | ?column? | sum
----+---+----------+----
- 1 | 4 |        5 |  9
- 2 | 5 |        7 | 12
- 3 | 6 |        9 | 15
-(3 rows)
-</screen>
-   </para>
-
    <indexterm>
     <primary>HAVING</primary>
    </indexterm>
index 8d5a751af7e9f3a03affe372d33fe9fa3aea988d..837bf6d78321e5178629c091defe9273f7a6af68 100644 (file)
@@ -37,7 +37,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac
     [ { * | <replaceable class="parameter">expression</replaceable> [ [ AS ] <replaceable class="parameter">output_name</replaceable> ] } [, ...] ]
     [ FROM <replaceable class="parameter">from_item</replaceable> [, ...] ]
     [ WHERE <replaceable class="parameter">condition</replaceable> ]
-    [ GROUP BY { ALL | [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</replaceable> [, ...] } ]
+    [ GROUP BY [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</replaceable> [, ...] ]
     [ HAVING <replaceable class="parameter">condition</replaceable> ]
     [ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceable class="parameter">window_definition</replaceable> ) [, ...] ]
     [ { UNION | INTERSECT | EXCEPT } [ ALL | DISTINCT ] <replaceable class="parameter">select</replaceable> ]
@@ -839,7 +839,7 @@ WHERE <replaceable class="parameter">condition</replaceable>
    <para>
     The optional <literal>GROUP BY</literal> clause has the general form
 <synopsis>
-GROUP BY { ALL | [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</replaceable> [, ...] }
+GROUP BY [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</replaceable> [, ...]
 </synopsis>
    </para>
 
@@ -857,15 +857,6 @@ GROUP BY { ALL | [ ALL | DISTINCT ] <replaceable class="parameter">grouping_elem
     input-column name rather than an output column name.
    </para>
 
-   <para>
-    The form <literal>GROUP BY ALL</literal> with no explicit
-    <replaceable class="parameter">grouping_elements</replaceable>
-    provided is equivalent to writing <literal>GROUP BY</literal> with the
-    numbers of all <command>SELECT</command> output columns that do not
-    contain either an aggregate function referring to the same query level or
-    a window function.
-   </para>
-
    <para>
     If any of <literal>GROUPING SETS</literal>, <literal>ROLLUP</literal> or
     <literal>CUBE</literal> are present as grouping elements, then the
index cbf865ff8383cc469f85397d024a8c4c0a8b1412..550ba69d5d18e6ace75e8fa072a20dbc282f3989 100644 (file)
@@ -27,7 +27,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac
     INTO [ TEMPORARY | TEMP | UNLOGGED ] [ TABLE ] <replaceable class="parameter">new_table</replaceable>
     [ FROM <replaceable class="parameter">from_item</replaceable> [, ...] ]
     [ WHERE <replaceable class="parameter">condition</replaceable> ]
-    [ GROUP BY { ALL | [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</replaceable> [, ...] } ]
+    [ GROUP BY [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</replaceable> [, ...] ]
     [ HAVING <replaceable class="parameter">condition</replaceable> ]
     [ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceable class="parameter">window_definition</replaceable> ) [, ...] ]
     [ { UNION | INTERSECT | EXCEPT } [ ALL | DISTINCT ] <replaceable class="parameter">select</replaceable> ]
index ea97d236ea8179a03bc29c1450d255d14207632d..562e4facd74f46ae3bfa7346ba5b1dc73c9b0645 100644 (file)
@@ -1811,14 +1811,12 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt,
 
        qry->groupClause = transformGroupClause(pstate,
                                                                                        stmt->groupClause,
-                                                                                       stmt->groupByAll,
                                                                                        &qry->groupingSets,
                                                                                        &qry->targetList,
                                                                                        qry->sortClause,
                                                                                        EXPR_KIND_GROUP_BY,
                                                                                        false /* allow SQL92 rules */ );
        qry->groupDistinct = stmt->groupDistinct;
-       qry->groupByAll = stmt->groupByAll;
 
        if (stmt->distinctClause == NIL)
        {
index ff4e1388c55007e6464e3e02d783f2c7f8018b47..e1e65c414db944edeac0d0f59ed52606535c60fa 100644 (file)
@@ -120,7 +120,6 @@ typedef struct SelectLimit
 typedef struct GroupClause
 {
        bool            distinct;
-       bool            all;
        List       *list;
 } GroupClause;
 
@@ -13751,7 +13750,6 @@ simple_select:
                                        n->whereClause = $6;
                                        n->groupClause = ($7)->list;
                                        n->groupDistinct = ($7)->distinct;
-                                       n->groupByAll = ($7)->all;
                                        n->havingClause = $8;
                                        n->windowClause = $9;
                                        $$ = (Node *) n;
@@ -13769,7 +13767,6 @@ simple_select:
                                        n->whereClause = $6;
                                        n->groupClause = ($7)->list;
                                        n->groupDistinct = ($7)->distinct;
-                                       n->groupByAll = ($7)->all;
                                        n->havingClause = $8;
                                        n->windowClause = $9;
                                        $$ = (Node *) n;
@@ -14267,24 +14264,14 @@ group_clause:
                                        GroupClause *n = palloc_object(GroupClause);
 
                                        n->distinct = $3 == SET_QUANTIFIER_DISTINCT;
-                                       n->all = false;
                                        n->list = $4;
                                        $$ = n;
                                }
-                       | GROUP_P BY ALL
-                               {
-                                       GroupClause *n = palloc_object(GroupClause);
-                                       n->distinct = false;
-                                       n->all = true;
-                                       n->list = NIL;
-                                       $$ = n;
-                               }
                        | /*EMPTY*/
                                {
                                        GroupClause *n = palloc_object(GroupClause);
 
                                        n->distinct = false;
-                                       n->all = false;
                                        n->list = NIL;
                                        $$ = n;
                                }
@@ -18707,7 +18694,6 @@ PLpgSQL_Expr: opt_distinct_clause opt_target_list
                                        n->whereClause = $4;
                                        n->groupClause = ($5)->list;
                                        n->groupDistinct = ($5)->distinct;
-                                       n->groupByAll = ($5)->all;
                                        n->havingClause = $6;
                                        n->windowClause = $7;
                                        n->sortClause = $8;
index 2f7333e6786ad29df15a454e360d7aa0ea17e3e8..485e33b9e5addf62cc2212a203ba7a81415e6164 100644 (file)
@@ -2746,9 +2746,6 @@ transformGroupingSet(List **flatresult,
  * GROUP BY items will be added to the targetlist (as resjunk columns)
  * if not already present, so the targetlist must be passed by reference.
  *
- * If GROUP BY ALL is specified, the groupClause will be inferred to be all
- * non-aggregate, non-window expressions in the targetlist.
- *
  * This is also used for window PARTITION BY clauses (which act almost the
  * same, but are always interpreted per SQL99 rules).
  *
@@ -2773,7 +2770,6 @@ transformGroupingSet(List **flatresult,
  *
  * pstate              ParseState
  * grouplist   clause to transform
- * groupByAll  is this a GROUP BY ALL statement?
  * groupingSets reference to list to contain the grouping set tree
  * targetlist  reference to TargetEntry list
  * sortClause  ORDER BY clause (SortGroupClause nodes)
@@ -2781,8 +2777,7 @@ transformGroupingSet(List **flatresult,
  * useSQL99            SQL99 rather than SQL92 syntax
  */
 List *
-transformGroupClause(ParseState *pstate, List *grouplist, bool groupByAll,
-                                        List **groupingSets,
+transformGroupClause(ParseState *pstate, List *grouplist, List **groupingSets,
                                         List **targetlist, List *sortClause,
                                         ParseExprKind exprKind, bool useSQL99)
 {
@@ -2793,61 +2788,6 @@ transformGroupClause(ParseState *pstate, List *grouplist, bool groupByAll,
        bool            hasGroupingSets = false;
        Bitmapset  *seen_local = NULL;
 
-       /* Handle GROUP BY ALL */
-       if (groupByAll)
-       {
-               /* There cannot have been any explicit grouplist items */
-               Assert(grouplist == NIL);
-
-               /* Iterate over targets, adding acceptable ones to the result list */
-               foreach_ptr(TargetEntry, tle, *targetlist)
-               {
-                       /* Ignore junk TLEs */
-                       if (tle->resjunk)
-                               continue;
-
-                       /*
-                        * TLEs containing aggregates are not okay to add to GROUP BY
-                        * (compare checkTargetlistEntrySQL92).  But the SQL standard
-                        * directs us to skip them, so it's fine.
-                        */
-                       if (pstate->p_hasAggs &&
-                               contain_aggs_of_level((Node *) tle->expr, 0))
-                               continue;
-
-                       /*
-                        * Likewise, TLEs containing window functions are not okay to add
-                        * to GROUP BY, and the SQL standard directs us to skip them.
-                        */
-                       if (pstate->p_hasWindowFuncs &&
-                               contain_windowfuncs((Node *) tle->expr))
-                               continue;
-
-                       /*
-                        * Otherwise, add the TLE to the result using default sort/group
-                        * semantics.  We specify the parse location as the TLE's
-                        * location, despite the comment for addTargetToGroupList
-                        * discouraging that.  The only other thing we could point to is
-                        * the ALL keyword, which seems unhelpful when there are multiple
-                        * TLEs.
-                        */
-                       result = addTargetToGroupList(pstate, tle,
-                                                                                 result, *targetlist,
-                                                                                 exprLocation((Node *) tle->expr));
-               }
-
-               /* If we found any acceptable targets, we're done */
-               if (result != NIL)
-                       return result;
-
-               /*
-                * Otherwise, the SQL standard says to treat it like "GROUP BY ()".
-                * Build a representation of that, and let the rest of this function
-                * handle it.
-                */
-               grouplist = list_make1(makeGroupingSet(GROUPING_SET_EMPTY, NIL, -1));
-       }
-
        /*
         * Recursively flatten implicit RowExprs. (Technically this is only needed
         * for GROUP BY, per the syntax rules for grouping sets, but we do it
@@ -3026,7 +2966,6 @@ transformWindowDefinitions(ParseState *pstate,
                                                                                  true /* force SQL99 rules */ );
                partitionClause = transformGroupClause(pstate,
                                                                                           windef->partitionClause,
-                                                                                          false /* not GROUP BY ALL */ ,
                                                                                           NULL,
                                                                                           targetlist,
                                                                                           orderClause,
index 819631781c0198919899a11e2228c60162150bd0..c6555948df8553a37a9e7022a1d0f2cbe1a856ca 100644 (file)
@@ -6555,9 +6555,7 @@ get_basic_select_query(Query *query, deparse_context *context)
                save_ingroupby = context->inGroupBy;
                context->inGroupBy = true;
 
-               if (query->groupByAll)
-                       appendStringInfoString(buf, "ALL");
-               else if (query->groupingSets == NIL)
+               if (query->groupingSets == NIL)
                {
                        sep = "";
                        foreach(l, query->groupClause)
index 79291aaaf5c1cb05a2e211376a3ecda2761ccc77..2471ccb96a35198946d759244e72b9b94b497886 100644 (file)
@@ -57,6 +57,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     202607071
+#define CATALOG_VERSION_NO     202607172
 
 #endif
index 4133c404a6bc0ec230543178dbc2215d0d1fd7ed..ad31a9c059bf5e7a5b0f8f011c07b507ce382d8a 100644 (file)
@@ -220,7 +220,6 @@ typedef struct Query
 
        List       *groupClause;        /* a list of SortGroupClause's */
        bool            groupDistinct;  /* was GROUP BY DISTINCT used? */
-       bool            groupByAll;             /* was GROUP BY ALL used? */
 
        List       *groupingSets;       /* a list of GroupingSet's if present */
 
@@ -2297,7 +2296,6 @@ typedef struct SelectStmt
        Node       *whereClause;        /* WHERE qualification */
        List       *groupClause;        /* GROUP BY clauses */
        bool            groupDistinct;  /* Is this GROUP BY DISTINCT? */
-       bool            groupByAll;             /* Is this GROUP BY ALL? */
        Node       *havingClause;       /* HAVING conditional-expression */
        List       *windowClause;       /* WINDOW window_name AS (...), ... */
 
index fe23461100755f413322e4b31060b3bcfcebc770..ca815a9d1bbe15e91695593679a0931f257db1df 100644 (file)
@@ -26,7 +26,6 @@ extern Node *transformLimitClause(ParseState *pstate, Node *clause,
                                                                  ParseExprKind exprKind, const char *constructName,
                                                                  LimitOption limitOption);
 extern List *transformGroupClause(ParseState *pstate, List *grouplist,
-                                                                 bool groupByAll,
                                                                  List **groupingSets,
                                                                  List **targetlist, List *sortClause,
                                                                  ParseExprKind exprKind, bool useSQL99);
index 728a3ecd03f89714666bed472f66a74ec2b19944..1824f7db55774cd9eb43092e072287bc14e08a89 100644 (file)
@@ -1774,129 +1774,6 @@ select a, count(*) from t_having group by a having a = row(1.0)::avg_rec;
 drop table t_having;
 drop type avg_rec;
 --
--- Test GROUP BY ALL
---
--- We don't care about the data here, just the proper transformation of the
--- GROUP BY clause, so test some queries and verify the EXPLAIN plans.
---
-CREATE TEMP TABLE t1 (
-  a int,
-  b int,
-  c int
-);
--- basic example
-EXPLAIN (COSTS OFF) SELECT b, COUNT(*) FROM t1 GROUP BY ALL;
-      QUERY PLAN      
-----------------------
- HashAggregate
-   Group Key: b
-   ->  Seq Scan on t1
-(3 rows)
-
--- multiple columns, non-consecutive order
-EXPLAIN (COSTS OFF) SELECT a, SUM(b), b FROM t1 GROUP BY ALL;
-      QUERY PLAN      
-----------------------
- HashAggregate
-   Group Key: a, b
-   ->  Seq Scan on t1
-(3 rows)
-
--- multi columns, no aggregate
-EXPLAIN (COSTS OFF) SELECT a + b FROM t1 GROUP BY ALL;
-      QUERY PLAN      
-----------------------
- HashAggregate
-   Group Key: (a + b)
-   ->  Seq Scan on t1
-(3 rows)
-
--- check we detect a non-top-level aggregate
-EXPLAIN (COSTS OFF) SELECT a, SUM(b) + 4 FROM t1 GROUP BY ALL;
-      QUERY PLAN      
-----------------------
- HashAggregate
-   Group Key: a
-   ->  Seq Scan on t1
-(3 rows)
-
--- including grouped column is okay
-EXPLAIN (COSTS OFF) SELECT a, SUM(b) + a FROM t1 GROUP BY ALL;
-      QUERY PLAN      
-----------------------
- HashAggregate
-   Group Key: a
-   ->  Seq Scan on t1
-(3 rows)
-
--- including non-grouped column, not so much
-EXPLAIN (COSTS OFF) SELECT a, SUM(b) + c FROM t1 GROUP BY ALL;
-ERROR:  column "t1.c" must appear in the GROUP BY clause or be used in an aggregate function
-LINE 1: EXPLAIN (COSTS OFF) SELECT a, SUM(b) + c FROM t1 GROUP BY AL...
-                                               ^
--- all aggregates, should reduce to GROUP BY ()
-EXPLAIN (COSTS OFF) SELECT COUNT(a), SUM(b) FROM t1 GROUP BY ALL;
-      QUERY PLAN      
-----------------------
- Aggregate
-   Group Key: ()
-   ->  Seq Scan on t1
-(3 rows)
-
--- likewise with empty target list
-EXPLAIN (COSTS OFF) SELECT FROM t1 GROUP BY ALL;
-      QUERY PLAN       
------------------------
- Result
-   Replaces: Aggregate
-(2 rows)
-
--- window functions are not to be included in GROUP BY, either
-EXPLAIN (COSTS OFF) SELECT a, COUNT(a) OVER (PARTITION BY a) FROM t1 GROUP BY ALL;
-            QUERY PLAN            
-----------------------------------
- WindowAgg
-   Window: w1 AS (PARTITION BY a)
-   ->  Sort
-         Sort Key: a
-         ->  HashAggregate
-               Group Key: a
-               ->  Seq Scan on t1
-(7 rows)
-
--- all cols
-EXPLAIN (COSTS OFF) SELECT *, count(*) FROM t1 GROUP BY ALL;
-      QUERY PLAN      
-----------------------
- HashAggregate
-   Group Key: a, b, c
-   ->  Seq Scan on t1
-(3 rows)
-
--- group by all with grouping element(s) (equivalent to GROUP BY's
--- default behavior, explicit antithesis to GROUP BY DISTINCT)
-EXPLAIN (COSTS OFF) SELECT a, count(*) FROM t1 GROUP BY ALL a;
-      QUERY PLAN      
-----------------------
- HashAggregate
-   Group Key: a
-   ->  Seq Scan on t1
-(3 rows)
-
--- verify deparsing of GROUP BY ALL
-CREATE TEMP VIEW v1 AS SELECT b, COUNT(*) FROM t1 GROUP BY ALL;
-SELECT pg_get_viewdef('v1'::regclass);
-    pg_get_viewdef     
------------------------
-  SELECT b,           +
-     count(*) AS count+
-    FROM t1           +
-   GROUP BY ALL;
-(1 row)
-
-DROP VIEW v1;
-DROP TABLE t1;
---
 -- Test GROUP BY matching of join columns that are type-coerced due to USING
 --
 create temp table t1(f1 int, f2 int);
index 342605d5497f58c247b94b25ea5cb223d13bb6f9..490c52d4c03f15e7a0fe316c90c202f8deb781a1 100644 (file)
@@ -640,60 +640,6 @@ select a, count(*) from t_having group by a having a = row(1.0)::avg_rec;
 drop table t_having;
 drop type avg_rec;
 
---
--- Test GROUP BY ALL
---
--- We don't care about the data here, just the proper transformation of the
--- GROUP BY clause, so test some queries and verify the EXPLAIN plans.
---
-
-CREATE TEMP TABLE t1 (
-  a int,
-  b int,
-  c int
-);
-
--- basic example
-EXPLAIN (COSTS OFF) SELECT b, COUNT(*) FROM t1 GROUP BY ALL;
-
--- multiple columns, non-consecutive order
-EXPLAIN (COSTS OFF) SELECT a, SUM(b), b FROM t1 GROUP BY ALL;
-
--- multi columns, no aggregate
-EXPLAIN (COSTS OFF) SELECT a + b FROM t1 GROUP BY ALL;
-
--- check we detect a non-top-level aggregate
-EXPLAIN (COSTS OFF) SELECT a, SUM(b) + 4 FROM t1 GROUP BY ALL;
-
--- including grouped column is okay
-EXPLAIN (COSTS OFF) SELECT a, SUM(b) + a FROM t1 GROUP BY ALL;
-
--- including non-grouped column, not so much
-EXPLAIN (COSTS OFF) SELECT a, SUM(b) + c FROM t1 GROUP BY ALL;
-
--- all aggregates, should reduce to GROUP BY ()
-EXPLAIN (COSTS OFF) SELECT COUNT(a), SUM(b) FROM t1 GROUP BY ALL;
-
--- likewise with empty target list
-EXPLAIN (COSTS OFF) SELECT FROM t1 GROUP BY ALL;
-
--- window functions are not to be included in GROUP BY, either
-EXPLAIN (COSTS OFF) SELECT a, COUNT(a) OVER (PARTITION BY a) FROM t1 GROUP BY ALL;
-
--- all cols
-EXPLAIN (COSTS OFF) SELECT *, count(*) FROM t1 GROUP BY ALL;
-
--- group by all with grouping element(s) (equivalent to GROUP BY's
--- default behavior, explicit antithesis to GROUP BY DISTINCT)
-EXPLAIN (COSTS OFF) SELECT a, count(*) FROM t1 GROUP BY ALL a;
-
--- verify deparsing of GROUP BY ALL
-CREATE TEMP VIEW v1 AS SELECT b, COUNT(*) FROM t1 GROUP BY ALL;
-SELECT pg_get_viewdef('v1'::regclass);
-
-DROP VIEW v1;
-DROP TABLE t1;
-
 --
 -- Test GROUP BY matching of join columns that are type-coerced due to USING
 --