Re: [HACKERS] [PATCH] Allow star syntax in GROUP BY, as a shorthand for all table columns

2011-06-09 Thread Marti Raudsepp
On Wed, Jun 8, 2011 at 20:22, Tom Lane  wrote:
> Is this really necessary now that we know about "GROUP BY primary key"?

You're right. I was just looking for something easy to hack on and
didn't put much thought into usefulness.
I'll try to do better next time. :)

Regards,
Marti

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [PATCH] Allow star syntax in GROUP BY, as a shorthand for all table columns

2011-06-08 Thread Tom Lane
Marti Raudsepp  writes:
> This patch enables the syntax "GROUP BY tablename.*" in cases where
> earlier you'd get the error "field must appear in the GROUP BY clause
> or be used in an aggregate function"

Is this really necessary now that we know about "GROUP BY primary key"?

> The patch is so trivial that I'm wondering why it hasn't been
> implemented before.

Probably because it's a nonstandard kluge ...

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] [PATCH] Allow star syntax in GROUP BY, as a shorthand for all table columns

2011-06-08 Thread Marti Raudsepp
Hi,

This patch enables the syntax "GROUP BY tablename.*" in cases where
earlier you'd get the error "field must appear in the GROUP BY clause
or be used in an aggregate function"

I've often needed to write queries like this:
  SELECT a.x, a.y, a.z, sum(b.w) FROM a JOIN b USING (a_id) GROUP BY
a.x, a.y, a.z;
Now this becomes:
  SELECT a.x, a.y, a.z, sum(b.w) FROM a JOIN b USING (a_id) GROUP BY a.*;

The patch is so trivial that I'm wondering why it hasn't been
implemented before. I couldn't think of any assumptions being broken
by using row comparison instead of comparing each field separately.
But maybe I'm missing something.

If this patch looks reasonable, I guess the obvious next step is to
expand the "a.*" reference to the table's primary key columns and fill
in context->func_grouped_rels

Regards,
Marti
From 36aa45fddae0623db4049484ac75533901bc69c9 Mon Sep 17 00:00:00 2001
From: Marti Raudsepp 
Date: Wed, 8 Jun 2011 19:21:16 +0300
Subject: [PATCH] Allow star syntax in GROUP BY, as a shorthand for all table
 columns

Marti Raudsepp
---
 src/backend/catalog/pg_constraint.c  |2 +-
 src/backend/parser/parse_agg.c   |2 +-
 src/test/regress/expected/aggregates.out |   21 +
 src/test/regress/sql/aggregates.sql  |6 ++
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 6997994..105e724 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -875,7 +875,7 @@ check_functional_grouping(Oid relid,
 if (IsA(gvar, Var) &&
 	gvar->varno == varno &&
 	gvar->varlevelsup == varlevelsup &&
-	gvar->varattno == attnum)
+	(gvar->varattno == attnum || gvar->varattno == 0))
 {
 	found_col = true;
 	break;
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 8356133..c75edab 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -623,7 +623,7 @@ check_ungrouped_columns_walker(Node *node,
 
 if (IsA(gvar, Var) &&
 	gvar->varno == var->varno &&
-	gvar->varattno == var->varattno &&
+	(gvar->varattno == var->varattno || gvar->varattno == 0) &&
 	gvar->varlevelsup == 0)
 	return false;		/* acceptable, we're okay */
 			}
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 4861006..418edc0 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -1061,3 +1061,24 @@ select string_agg(distinct f1::text, ',' order by f1::text) from varchar_tbl;  -
  a,ab,abcd
 (1 row)
 
+-- test GROUP BY using table/star syntax
+select sin(b), * from aggtest group by aggtest.*;
+sin |  a  |b
++-+-
+ 0.0954644005855398 |   0 | 0.09561
+  -0.93056589608634 |  42 |  324.78
+  0.998543355665767 |  56 | 7.8
+ -0.990653162615688 | 100 |  99.097
+(4 rows)
+
+select count(*), t1.* from aggtest t1 cross join aggtest t2 group by t1;
+ count |  a  |b
+---+-+-
+ 4 |   0 | 0.09561
+ 4 |  42 |  324.78
+ 4 |  56 | 7.8
+ 4 | 100 |  99.097
+(4 rows)
+
+select * from aggtest t1 cross join aggtest t2 group by t1.*;
+ERROR:  column "t2.a" must appear in the GROUP BY clause or be used in an aggregate function
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 04ec67b..dc59750 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -416,3 +416,9 @@ select string_agg(distinct f1, ',' order by f1) from varchar_tbl;  -- ok
 select string_agg(distinct f1::text, ',' order by f1) from varchar_tbl;  -- not ok
 select string_agg(distinct f1, ',' order by f1::text) from varchar_tbl;  -- not ok
 select string_agg(distinct f1::text, ',' order by f1::text) from varchar_tbl;  -- ok
+
+-- test GROUP BY using table/star syntax
+select sin(b), * from aggtest group by aggtest.*;
+select count(*), t1.* from aggtest t1 cross join aggtest t2 group by t1;
+select * from aggtest t1 cross join aggtest t2 group by t1.*;
+
-- 
1.7.5.4


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers