Attribute must be GROUPed.... ? - Mailing list pgsql-hackers

From Daniele Orlandi
Subject Attribute must be GROUPed.... ?
Date
Msg-id 3EB03E59.8060102@orlandi.com
Whole thread Raw
Responses Re: Attribute must be GROUPed.... ?  (Stephan Szabo <sszabo@megazone23.bigpanda.com>)
List pgsql-hackers
I always had been doubious about the "must" in the error message
"Attribute must be GROUPed or used in an aggregate function".

I certainly agree that there are many situations in which having an
ungrouped attribute in the target list is nonsense (undefined), but I
sometimes find situations in which it would be perfetcly meaningful.

For example, suppose that you join two tables and the field you group by
appears to be the primary key of one of the tables. In this case if I
put any of the fields in that table in the target list, it will always
be well defined and would make certain queries much simpler.

In SQL:

# create table cities (id int primary key,name text);
# create table people (fullname text,city_id int references cities(id));
# select name,count(*) from people,cities where people.city_id=cities.id
group by city_id;

   name      |    count
------------+-------------
New York    | 10
Los Angeles | 15
Seveso      | 1
....

Here follows a patch that makes this behaviour configurable via a
configuration variable. I apologize in advance if there's something
stupid, it's my first patch on postgresql...

The default is the current behaviour but I would propose to change the
default to be more permissive.

Bye!

--
   Daniele Orlandi
   Planet Srl

Index: src/backend/parser/parse_agg.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/parser/parse_agg.c,v
retrieving revision 1.52
diff -u -r1.52 parse_agg.c
--- src/backend/parser/parse_agg.c    2003/04/03 18:04:09    1.52
+++ src/backend/parser/parse_agg.c    2003/04/30 20:39:29
@@ -34,6 +34,8 @@
 static bool check_ungrouped_columns_walker(Node *node,
                                check_ungrouped_columns_context *context);

+bool force_targets_grouped = true;
+
 /*
  * check_ungrouped_columns -
  *      Scan the given expression tree for ungrouped variables (variables
@@ -246,13 +248,21 @@
                                                         (Node *) groupClauses);

     /*
-     * Check the targetlist and HAVING clause for ungrouped variables.
+     * Check the targetlist clause for ungrouped variables.
      */
-    clause = (Node *) qry->targetList;
-    if (hasJoinRTEs)
-        clause = flatten_join_alias_vars(qry, clause);
-    check_ungrouped_columns(clause, pstate,
-                            groupClauses, have_non_var_grouping);
+
+    if (force_targets_grouped)
+    {
+        clause = (Node *) qry->targetList;
+        if (hasJoinRTEs)
+            clause = flatten_join_alias_vars(qry, clause);
+        check_ungrouped_columns(clause, pstate,
+                                groupClauses, have_non_var_grouping);
+    }
+
+    /*
+     * Check the HAVING clause for ungrouped variables.
+     */

     clause = (Node *) qry->havingQual;
     if (hasJoinRTEs)
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.119
diff -u -r1.119 guc.c
--- src/backend/utils/misc/guc.c    2003/04/25 19:45:09    1.119
+++ src/backend/utils/misc/guc.c    2003/04/30 20:39:31
@@ -43,6 +43,7 @@
 #include "optimizer/paths.h"
 #include "optimizer/prep.h"
 #include "parser/parse_expr.h"
+#include "parser/parse_agg.h"
 #include "storage/fd.h"
 #include "storage/freespace.h"
 #include "storage/lock.h"
@@ -533,6 +534,10 @@
     {
         {"transaction_read_only", PGC_USERSET, GUC_NO_RESET_ALL}, &XactReadOnly,
         false, NULL, NULL
+    },
+    {
+        {"force_targets_grouped", PGC_USERSET, GUC_NO_RESET_ALL}, &force_targets_grouped,
+        true, NULL, NULL
     },

     {
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.77
diff -u -r1.77 postgresql.conf.sample
--- src/backend/utils/misc/postgresql.conf.sample    2003/04/19 00:37:28    1.77
+++ src/backend/utils/misc/postgresql.conf.sample    2003/04/30 20:39:31
@@ -219,3 +219,4 @@
 #statement_timeout = 0        # 0 is disabled, in milliseconds
 #db_user_namespace = false
 #preload_libraries = ''
+#force_targets_grouped = true
Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.76
diff -u -r1.76 tab-complete.c
--- src/bin/psql/tab-complete.c    2003/04/03 20:18:16    1.76
+++ src/bin/psql/tab-complete.c    2003/04/30 20:39:31
@@ -522,6 +522,7 @@
         "enable_tidscan",
         "explain_pretty_print",
         "extra_float_digits",
+        "force_targets_grouped",
         "from_collapse_limit",
         "fsync",
         "geqo",
Index: src/include/parser/parse_agg.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/include/parser/parse_agg.h,v
retrieving revision 1.25
diff -u -r1.25 parse_agg.h
--- src/include/parser/parse_agg.h    2003/01/17 03:25:04    1.25
+++ src/include/parser/parse_agg.h    2003/04/30 20:39:32
@@ -17,4 +17,6 @@

 extern void parseCheckAggregates(ParseState *pstate, Query *qry);

+extern bool force_targets_grouped;
+
 #endif   /* PARSE_AGG_H */


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Planned changes in backend memory management
Next
From: Stephan Szabo
Date:
Subject: Re: Attribute must be GROUPed.... ?