Re: [HACKERS] GROUPING SETS revisited

2010-08-18 Thread Pavel Stehule
Hello

I found a break in GROUPING SETS implementation. Now I am playing with
own executor and planner node and I can't to go forward :(. Probably
this feature will need a significant update of our agg implementation.
Probably needs a some similar structure like CTE but it can be a
little bit reduced - there are a simple relation between source query
and result query - I am not sure, if this has to be implemented via
subqueries? The second question is relative big differencies between
GROUP BY behave and GROUP BY GROUPING SETS behave. Now I don't know
about way to join GROUP BY and GROUPING SETS together

Any ideas welcome

Regards

Pavel

-- 
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] GROUPING SETS revisited

2010-08-09 Thread Pavel Stehule
Hello

I was confused when I though so I found a solution of 1 shift/reduce conflict :(

All identificators used for buildin functions have to be a
col_name_keywords or reserved keyword. There is conflict with our
(probably obsolete) feature SELECT colname(tabname). So for this
moment the real solution is removing CUBE and ROLLUP from keywords and
dynamically testing a funcname in transformation stage - what is
slower and more ugly.

ideas?

Regards

Pavel Stehule


2010/8/7 Pavel Stehule pavel.steh...@gmail.com:
 2010/8/7 Joshua Tolley eggyk...@gmail.com:
 On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
 I am sending a updated version.

 I've been looking at the changes to gram.y, and noted the comment under 
 func_expr
 where you added CUBE and ROLLUP definitions. It says that CUBE can't be a
 reserved keyword because it's already used in the cube contrib module. But
 then the changes to kwlist.h include this:


 I am little bit confused now - it's bad comment - and I have to verify
 it. What I remember, we cannot to use a two parser's rules, because it
 going to a conflict. So there have to be used a trick with a moving to
 decision to transform stage, where we have a context info. I have to
 recheck a minimal level - probably it can't be a RESERVED_KEYWORD.
 Because then we can't to create a function cube.

 + PG_KEYWORD(cube, CUBE, RESERVED_KEYWORD)
 ...
 + PG_KEYWORD(rollup, ROLLUP, RESERVED_KEYWORD)

 ...and CUBE and ROLLUP are added in gram.y under the reserved_keyword list. I
 realize things like CURRENT_TIME, that also have special entries in the
 func_expr grammar, are also reserved keywords, but this all seems at odds 
 with
 the comment. What am I missing? Is the comment simply pointing out that the
 designation of CUBE and ROLLUP as reserved keywords will have to change at
 some point, but it hasn't been implemented yet (or no one has figured out how
 to do it)?

 --
 Joshua Tolley / eggyknap
 End Point Corporation
 http://www.endpoint.com

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAkxcjSIACgkQRiRfCGf1UMPpCwCcCHBh/1NiLykIcVYgPyfbIegF
 xq0AoID75rCPiW8yf29OSkaJVza1FQt5
 =PcLs
 -END PGP SIGNATURE-




-- 
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] GROUPING SETS revisited

2010-08-06 Thread Joshua Tolley
On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
 I am sending a updated version.

I've been looking at the changes to gram.y, and noted the comment under 
func_expr
where you added CUBE and ROLLUP definitions. It says that CUBE can't be a
reserved keyword because it's already used in the cube contrib module. But
then the changes to kwlist.h include this:

+ PG_KEYWORD(cube, CUBE, RESERVED_KEYWORD)
...
+ PG_KEYWORD(rollup, ROLLUP, RESERVED_KEYWORD)

...and CUBE and ROLLUP are added in gram.y under the reserved_keyword list. I
realize things like CURRENT_TIME, that also have special entries in the
func_expr grammar, are also reserved keywords, but this all seems at odds with
the comment. What am I missing? Is the comment simply pointing out that the
designation of CUBE and ROLLUP as reserved keywords will have to change at
some point, but it hasn't been implemented yet (or no one has figured out how
to do it)?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-06 Thread Pavel Stehule
2010/8/7 Joshua Tolley eggyk...@gmail.com:
 On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
 I am sending a updated version.

 I've been looking at the changes to gram.y, and noted the comment under 
 func_expr
 where you added CUBE and ROLLUP definitions. It says that CUBE can't be a
 reserved keyword because it's already used in the cube contrib module. But
 then the changes to kwlist.h include this:


I am little bit confused now - it's bad comment - and I have to verify
it. What I remember, we cannot to use a two parser's rules, because it
going to a conflict. So there have to be used a trick with a moving to
decision to transform stage, where we have a context info. I have to
recheck a minimal level - probably it can't be a RESERVED_KEYWORD.
Because then we can't to create a function cube.

 + PG_KEYWORD(cube, CUBE, RESERVED_KEYWORD)
 ...
 + PG_KEYWORD(rollup, ROLLUP, RESERVED_KEYWORD)

 ...and CUBE and ROLLUP are added in gram.y under the reserved_keyword list. I
 realize things like CURRENT_TIME, that also have special entries in the
 func_expr grammar, are also reserved keywords, but this all seems at odds with
 the comment. What am I missing? Is the comment simply pointing out that the
 designation of CUBE and ROLLUP as reserved keywords will have to change at
 some point, but it hasn't been implemented yet (or no one has figured out how
 to do it)?

 --
 Joshua Tolley / eggyknap
 End Point Corporation
 http://www.endpoint.com

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAkxcjSIACgkQRiRfCGf1UMPpCwCcCHBh/1NiLykIcVYgPyfbIegF
 xq0AoID75rCPiW8yf29OSkaJVza1FQt5
 =PcLs
 -END PGP SIGNATURE-



-- 
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] GROUPING SETS revisited

2010-08-05 Thread Joshua Tolley
On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
 So Joshua, can you look on code?

Sure... thanks :)

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-05 Thread Pavel Stehule
I found other issue :(

postgres=#  select name, place from cars group by grouping sets(name, place,());
 name  |   place
---+
 bmw   |
 skoda |
 opel  |
   | germany
   | czech rep.
 skoda | czech rep.
 skoda | germany
 bmw   | czech rep.
 bmw   | germany
 opel  | czech rep.
 opel  | germany
(11 rows)

postgres=# explain select name, place from cars group by grouping
sets(name, place,());
  QUERY PLAN
--
 Append  (cost=36.98..88.55 rows=1230 width=54)
   CTE GroupingSets
 -  Seq Scan on cars  (cost=0.00..18.30 rows=830 width=68)
   -  HashAggregate  (cost=18.68..20.68 rows=200 width=32)
 -  CTE Scan on GroupingSets  (cost=0.00..16.60 rows=830 width=32)
   -  HashAggregate  (cost=18.68..20.68 rows=200 width=32)
 -  CTE Scan on GroupingSets  (cost=0.00..16.60 rows=830 width=32)
   -  CTE Scan on GroupingSets  (cost=0.00..16.60 rows=830 width=64)
(8 rows)

the combination of nonagregates and empty sets do a problems - because
we can't ensure agg mode without aggregates or group by. But it is
only minor issue

2010/8/5 Joshua Tolley eggyk...@gmail.com:
 On Thu, Aug 05, 2010 at 04:46:51PM +0200, Pavel Stehule wrote:
 So Joshua, can you look on code?

 Sure... thanks :)

 --
 Joshua Tolley / eggyknap
 End Point Corporation
 http://www.endpoint.com

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAkxa1NsACgkQRiRfCGf1UMPwzQCgjz52P86Yx4ac4aRkKwjn8OHK
 6/EAoJ/CjXEyPaLpx39SI5bKQPz+AwBR
 =Mi2J
 -END PGP SIGNATURE-



-- 
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] GROUPING SETS revisited

2010-08-04 Thread Joshua Tolley
On Wed, Aug 04, 2010 at 04:44:05AM +0200, Pavel Stehule wrote:
  Yeah, I seem to have done a poor job of producing the patch based on the
  repository I was working from. That said, it seems Pavel's working actively 
  on
  a patch anyway, so perhaps my updating the old one isn't all that 
  worthwhile.
  Pavel, is your code somewhere that we can get to it?
 
 
 not now. please wait a week.

That works for me. I'm glad to try doing a better job of putting together my
version of the patch, if anyone thinks it's useful, but it seems that since
Pavel's code is due to appear sometime in the foreseeable future, there's not
much point in my doing that.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-04 Thread Pavel Stehule
2010/8/4 Joshua Tolley eggyk...@gmail.com:
 On Wed, Aug 04, 2010 at 04:44:05AM +0200, Pavel Stehule wrote:
  Yeah, I seem to have done a poor job of producing the patch based on the
  repository I was working from. That said, it seems Pavel's working 
  actively on
  a patch anyway, so perhaps my updating the old one isn't all that 
  worthwhile.
  Pavel, is your code somewhere that we can get to it?
 

 not now. please wait a week.

 That works for me. I'm glad to try doing a better job of putting together my
 version of the patch, if anyone thinks it's useful, but it seems that since
 Pavel's code is due to appear sometime in the foreseeable future, there's not
 much point in my doing that.


I hope, so next week you can do own work on this job - I am not a
native speaker, and my code will need a checking and fixing comments

Regards

Pavel

 --
 Joshua Tolley / eggyknap
 End Point Corporation
 http://www.endpoint.com

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAkxZkp8ACgkQRiRfCGf1UMMUcwCfcPayQbWRUYwhpCF1f24LsdD9
 H/gAnRzCEq6yLX/RVLLi88ROhurOzbhK
 =gUPx
 -END PGP SIGNATURE-



-- 
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] GROUPING SETS revisited

2010-08-04 Thread Joshua Tolley
On Thu, Aug 05, 2010 at 06:21:18AM +0200, Pavel Stehule wrote:
 I hope, so next week you can do own work on this job - I am not a
 native speaker, and my code will need a checking and fixing comments

I haven't entirely figured out how the code in the old patch works, but I
promise I *can* edit comments/docs :)

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-03 Thread Pavel Stehule
Hello

2010/8/3 Joshua Tolley eggyk...@gmail.com:
 In case anyone's interested, I've taken the CTE-based grouping sets patch from
 [1] and made it apply to 9.1, attached. I haven't yet done things like checked
 it for whitespace consistency, style conformity, or anything else, but (tuits
 permitting) hope to figure out how it works and get it closer to commitability
 in some upcoming commitfest.

 I mention it here so that if someone else is working on it, we can avoid
 duplicated effort, and to see if a CTE-based grouping sets implementation is
 really the way we think we want to go.


I am plaing with it now :). I have a plan to replace CTE with similar
but explicit executor node. The main issue of existing patch was using
just transformation to CTE. I agree, so it isn't too much extensiable
in future. Now I am cleaning identifiers little bit. Any colaboration
is welcome.

My plan:
1) clean CTE patch
2) replace CTE with explicit executor node, but still based on tuplestore
3) when will be possible parallel processing based on hash agg - then
we don't need to use tuplestore

comments??

Regards

Pavel

 [1] http://archives.postgresql.org/pgsql-hackers/2009-05/msg00700.php

 --
 Joshua Tolley / eggyknap
 End Point Corporation
 http://www.endpoint.com

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAkxXrggACgkQRiRfCGf1UMMlCQCglaIdtPj8Qe6G60V2LHn5pFNn
 kgIAniXRgIQEbVrK/eDVZnmKCzw33lT9
 =XVVV
 -END PGP SIGNATURE-



-- 
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] GROUPING SETS revisited

2010-08-03 Thread Hitoshi Harada
2010/8/3 Pavel Stehule pavel.steh...@gmail.com:
 Hello

 2010/8/3 Joshua Tolley eggyk...@gmail.com:
 In case anyone's interested, I've taken the CTE-based grouping sets patch 
 from
 [1] and made it apply to 9.1, attached. I haven't yet done things like 
 checked
 it for whitespace consistency, style conformity, or anything else, but (tuits
 permitting) hope to figure out how it works and get it closer to 
 commitability
 in some upcoming commitfest.

 I mention it here so that if someone else is working on it, we can avoid
 duplicated effort, and to see if a CTE-based grouping sets implementation is
 really the way we think we want to go.


 I am plaing with it now :). I have a plan to replace CTE with similar
 but explicit executor node. The main issue of existing patch was using
 just transformation to CTE. I agree, so it isn't too much extensiable
 in future. Now I am cleaning identifiers little bit. Any colaboration
 is welcome.

 My plan:
 1) clean CTE patch
 2) replace CTE with explicit executor node, but still based on tuplestore
 3) when will be possible parallel processing based on hash agg - then
 we don't need to use tuplestore

Couldn't you explain what exactly explicit executor node? I hope we
can share your image to develop it further than only transformation to
CTE.


Regards,

-- 
Hitoshi Harada

-- 
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] GROUPING SETS revisited

2010-08-03 Thread Pavel Stehule
2010/8/3 Hitoshi Harada umi.tan...@gmail.com:
 2010/8/3 Pavel Stehule pavel.steh...@gmail.com:
 Hello

 2010/8/3 Joshua Tolley eggyk...@gmail.com:
 In case anyone's interested, I've taken the CTE-based grouping sets patch 
 from
 [1] and made it apply to 9.1, attached. I haven't yet done things like 
 checked
 it for whitespace consistency, style conformity, or anything else, but 
 (tuits
 permitting) hope to figure out how it works and get it closer to 
 commitability
 in some upcoming commitfest.

 I mention it here so that if someone else is working on it, we can avoid
 duplicated effort, and to see if a CTE-based grouping sets implementation is
 really the way we think we want to go.


 I am plaing with it now :). I have a plan to replace CTE with similar
 but explicit executor node. The main issue of existing patch was using
 just transformation to CTE. I agree, so it isn't too much extensiable
 in future. Now I am cleaning identifiers little bit. Any colaboration
 is welcome.

 My plan:
 1) clean CTE patch
 2) replace CTE with explicit executor node, but still based on tuplestore
 3) when will be possible parallel processing based on hash agg - then
 we don't need to use tuplestore

 Couldn't you explain what exactly explicit executor node? I hope we
 can share your image to develop it further than only transformation to
 CTE.

I have a one reason

Implementation based on CTE doesn't create space for possible
optimalisations (I think now, maybe it isn't true). It is good for
initial or referencial implementation - but it can be too complex,
when we will try to append some optimalizations - like parallel hash
agg processing, direct data reading without tuplestore. If you are, as
CTE author, thinking so these features are possible in non recursive
CTE too, I am not agains. I hope so this week I'll have a CTE based
patch - and we can talk about next direction. I see as possible
performance issue using a tuplestore - there are lot of cases where
repeating of source query can be faster.

If I remember well, Tom has a objection, so transformation to CTE is
too early - in parser. So It will be first change. Executor node can
be CTE.

regards

Pavel

p.s. I am sure, so there are lot of task, that can be solved together
with non recursive CTE.



 Regards,

 --
 Hitoshi Harada


-- 
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] GROUPING SETS revisited

2010-08-03 Thread David Fetter
On Mon, Aug 02, 2010 at 11:50:00PM -0600, Josh Tolley wrote:
 In case anyone's interested, I've taken the CTE-based grouping sets
 patch from [1] and made it apply to 9.1, attached. I haven't yet
 done things like checked it for whitespace consistency, style
 conformity, or anything else, but (tuits permitting) hope to figure
 out how it works and get it closer to commitability in some upcoming
 commitfest.
 
 I mention it here so that if someone else is working on it, we can
 avoid duplicated effort, and to see if a CTE-based grouping sets
 implementation is really the way we think we want to go.
 
 [1]
 http://archives.postgresql.org/pgsql-hackers/2009-05/msg00700.php

I've added back one file in the patch enclosed here.  I'm still
getting compile fails from

CC=ccache gcc ./configure --prefix=$PG_PREFIX --with-pgport=$PGPORT   
  --with-perl --with-libxml --enable-debug --enable-cassert
make

Log from that also enclosed.

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/src/backend/parser/Makefile b/src/backend/parser/Makefile
index a8f4c07..fb248a6 100644
--- a/src/backend/parser/Makefile
+++ b/src/backend/parser/Makefile
@@ -15,7 +15,7 @@ override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
 OBJS= analyze.o gram.o keywords.o kwlookup.o parser.o \
   parse_agg.o parse_clause.o parse_coerce.o parse_cte.o parse_expr.o \
   parse_func.o parse_node.o parse_oper.o parse_param.o parse_relation.o \
-  parse_target.o parse_type.o parse_utilcmd.o scansup.o
+  parse_target.o parse_type.o parse_utilcmd.o scansup.o parse_gsets.o
 
 FLEXFLAGS = -CF
 
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 6b99a10..1b579a8 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -34,6 +34,7 @@
 #include parser/parse_clause.h
 #include parser/parse_coerce.h
 #include parser/parse_cte.h
+#include parser/parse_gsets.h
 #include parser/parse_oper.h
 #include parser/parse_param.h
 #include parser/parse_relation.h
@@ -150,6 +151,163 @@ parse_sub_analyze(Node *parseTree, ParseState 
*parentParseState,
 }
 
 /*
+ * process GROUPING SETS
+ */
+static SelectStmt *
+makeSelectStmt(List *targetList, List *fromClause)
+{
+   SelectStmt *n = makeNode(SelectStmt);
+   n-distinctClause = NULL;
+   n-intoClause = NULL;
+   n-targetList = targetList;
+   n-fromClause = fromClause;
+   n-whereClause = NULL;
+   n-groupClause = NULL;
+   n-havingClause = NULL;
+   n-windowClause = NIL;
+   n-withClause = NULL;
+   n-valuesLists = NIL;
+   n-sortClause = NIL;
+   n-limitOffset = NULL;
+   n-limitCount = NULL;
+   n-lockingClause = NIL;
+   n-op = SETOP_NONE;
+   n-all = false;
+   n-larg = NULL;
+   n-rarg = NULL;
+   return n;
+}
+
+static List *
+makeStarTargetList(void)
+{
+   ResTarget *rt = makeNode(ResTarget);
+   
+   rt-name = NULL;
+   rt-indirection = NIL;
+   rt-val = (Node *) makeNode(ColumnRef);
+   ((ColumnRef *) rt-val)-fields = list_make1(makeNode(A_Star));
+   rt-location = -1;
+
+   return list_make1(rt);
+}
+ 
+static SelectStmt *
+transformGroupingSets(ParseState *pstate, SelectStmt *stmt)
+{
+   if (stmt-groupClause  IsA(stmt-groupClause, GroupByClause))
+   {
+   GroupingSetsSpec *gss = (GroupingSetsSpec *) 
expandGroupingSets(pstate, 
+   (List *)((GroupByClause 
*)stmt-groupClause)-fields);
+   
+   if (pstate-p_hasGroupingSets)
+   {
+   CommonTableExpr *cte = makeNode(CommonTableExpr);
+   SelectStmt  *cteedstmt;
+   int ngroupingsets = list_length(gss-set_list) + 
(gss-has_empty_set ? 1 : 0);
+   boolall = ((GroupByClause *) 
stmt-groupClause)-all;
+   
+   cteedstmt = makeSelectStmt(NIL, NIL);
+   cteedstmt-intoClause = stmt-intoClause;
+   cteedstmt-sortClause = stmt-sortClause;
+   cteedstmt-limitOffset = stmt-limitOffset;
+   cteedstmt-limitCount = stmt-limitCount;
+   cteedstmt-lockingClause = stmt-lockingClause;
+   
+   cte-ctename = **g**;
+   cte-ctequery = (Node *) stmt;
+   cte-location = -1;
+   
+   cteedstmt-withClause = makeNode(WithClause);
+   cteedstmt-withClause-ctes = list_make1(cte);
+   cteedstmt-withClause-recursive = false;
+   

Re: [HACKERS] GROUPING SETS revisited

2010-08-03 Thread Joshua Tolley
On Tue, Aug 03, 2010 at 12:58:03PM -0700, David Fetter wrote:
 On Mon, Aug 02, 2010 at 11:50:00PM -0600, Josh Tolley wrote:
  In case anyone's interested, I've taken the CTE-based grouping sets
  patch from [1] and made it apply to 9.1, attached. I haven't yet
  done things like checked it for whitespace consistency, style
  conformity, or anything else, but (tuits permitting) hope to figure
  out how it works and get it closer to commitability in some upcoming
  commitfest.
  
  I mention it here so that if someone else is working on it, we can
  avoid duplicated effort, and to see if a CTE-based grouping sets
  implementation is really the way we think we want to go.
  
  [1]
  http://archives.postgresql.org/pgsql-hackers/2009-05/msg00700.php
 
 I've added back one file in the patch enclosed here.  I'm still
 getting compile fails from
 
 CC=ccache gcc ./configure --prefix=$PG_PREFIX --with-pgport=$PGPORT 
 --with-perl --with-libxml --enable-debug --enable-cassert
 make
 
 Log from that also enclosed.
 

Yeah, I seem to have done a poor job of producing the patch based on the
repository I was working from. That said, it seems Pavel's working actively on
a patch anyway, so perhaps my updating the old one isn't all that worthwhile.
Pavel, is your code somewhere that we can get to it?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


signature.asc
Description: Digital signature


Re: [HACKERS] GROUPING SETS revisited

2010-08-03 Thread Pavel Stehule
2010/8/3 Joshua Tolley eggyk...@gmail.com:
 On Tue, Aug 03, 2010 at 12:58:03PM -0700, David Fetter wrote:
 On Mon, Aug 02, 2010 at 11:50:00PM -0600, Josh Tolley wrote:
  In case anyone's interested, I've taken the CTE-based grouping sets
  patch from [1] and made it apply to 9.1, attached. I haven't yet
  done things like checked it for whitespace consistency, style
  conformity, or anything else, but (tuits permitting) hope to figure
  out how it works and get it closer to commitability in some upcoming
  commitfest.
 
  I mention it here so that if someone else is working on it, we can
  avoid duplicated effort, and to see if a CTE-based grouping sets
  implementation is really the way we think we want to go.
 
  [1]
  http://archives.postgresql.org/pgsql-hackers/2009-05/msg00700.php

 I've added back one file in the patch enclosed here.  I'm still
 getting compile fails from

 CC=ccache gcc ./configure     --prefix=$PG_PREFIX     
 --with-pgport=$PGPORT     --with-perl     --with-libxml     --enable-debug   
   --enable-cassert
 make

 Log from that also enclosed.


 Yeah, I seem to have done a poor job of producing the patch based on the
 repository I was working from. That said, it seems Pavel's working actively on
 a patch anyway, so perhaps my updating the old one isn't all that worthwhile.
 Pavel, is your code somewhere that we can get to it?


not now. please wait a week.

Regards

Pavel

 --
 Joshua Tolley / eggyknap
 End Point Corporation
 http://www.endpoint.com

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAkxYeiQACgkQRiRfCGf1UMPlEQCff+I4sCGtR+lzUs6Wb5JKi7Uu
 3qYAnjLHzHzyMSHHX55QsphkaBbEJ0Zf
 =uRqV
 -END PGP SIGNATURE-



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


[HACKERS] GROUPING SETS revisited

2010-08-02 Thread Joshua Tolley
In case anyone's interested, I've taken the CTE-based grouping sets patch from
[1] and made it apply to 9.1, attached. I haven't yet done things like checked
it for whitespace consistency, style conformity, or anything else, but (tuits
permitting) hope to figure out how it works and get it closer to commitability
in some upcoming commitfest.

I mention it here so that if someone else is working on it, we can avoid
duplicated effort, and to see if a CTE-based grouping sets implementation is
really the way we think we want to go.

[1] http://archives.postgresql.org/pgsql-hackers/2009-05/msg00700.php

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com
diff --git a/src/backend/parser/Makefile b/src/backend/parser/Makefile
index a8f4c07..fb248a6 100644
*** a/src/backend/parser/Makefile
--- b/src/backend/parser/Makefile
*** override CPPFLAGS := -I. -I$(srcdir) $(C
*** 15,21 
  OBJS= analyze.o gram.o keywords.o kwlookup.o parser.o \
parse_agg.o parse_clause.o parse_coerce.o parse_cte.o parse_expr.o \
parse_func.o parse_node.o parse_oper.o parse_param.o parse_relation.o \
!   parse_target.o parse_type.o parse_utilcmd.o scansup.o
  
  FLEXFLAGS = -CF
  
--- 15,21 
  OBJS= analyze.o gram.o keywords.o kwlookup.o parser.o \
parse_agg.o parse_clause.o parse_coerce.o parse_cte.o parse_expr.o \
parse_func.o parse_node.o parse_oper.o parse_param.o parse_relation.o \
!   parse_target.o parse_type.o parse_utilcmd.o scansup.o parse_gsets.o
  
  FLEXFLAGS = -CF
  
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 6b99a10..1b579a8 100644
*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
***
*** 34,39 
--- 34,40 
  #include parser/parse_clause.h
  #include parser/parse_coerce.h
  #include parser/parse_cte.h
+ #include parser/parse_gsets.h
  #include parser/parse_oper.h
  #include parser/parse_param.h
  #include parser/parse_relation.h
*** parse_sub_analyze(Node *parseTree, Parse
*** 150,155 
--- 151,313 
  }
  
  /*
+  * process GROUPING SETS
+  */
+ static SelectStmt *
+ makeSelectStmt(List *targetList, List *fromClause)
+ {
+ 	SelectStmt *n = makeNode(SelectStmt);
+ 	n-distinctClause = NULL;
+ 	n-intoClause = NULL;
+ 	n-targetList = targetList;
+ 	n-fromClause = fromClause;
+ 	n-whereClause = NULL;
+ 	n-groupClause = NULL;
+ 	n-havingClause = NULL;
+ 	n-windowClause = NIL;
+ 	n-withClause = NULL;
+ 	n-valuesLists = NIL;
+ 	n-sortClause = NIL;
+ 	n-limitOffset = NULL;
+ 	n-limitCount = NULL;
+ 	n-lockingClause = NIL;
+ 	n-op = SETOP_NONE;
+ 	n-all = false;
+ 	n-larg = NULL;
+ 	n-rarg = NULL;
+ 	return n;
+ }
+ 
+ static List *
+ makeStarTargetList(void)
+ {
+ 	ResTarget *rt = makeNode(ResTarget);
+ 	
+ 	rt-name = NULL;
+ 	rt-indirection = NIL;
+ 	rt-val = (Node *) makeNode(ColumnRef);
+ 	((ColumnRef *) rt-val)-fields = list_make1(makeNode(A_Star));
+ 	rt-location = -1;
+ 
+ 	return list_make1(rt);
+ }
+  
+ static SelectStmt *
+ transformGroupingSets(ParseState *pstate, SelectStmt *stmt)
+ {
+ 	if (stmt-groupClause  IsA(stmt-groupClause, GroupByClause))
+ 	{
+ 		GroupingSetsSpec *gss = (GroupingSetsSpec *) expandGroupingSets(pstate, 
+ 		(List *)((GroupByClause *)stmt-groupClause)-fields);
+ 	
+ 		if (pstate-p_hasGroupingSets)
+ 		{
+ 			CommonTableExpr *cte = makeNode(CommonTableExpr);
+ 			SelectStmt  *cteedstmt;
+ 			int	ngroupingsets = list_length(gss-set_list) + (gss-has_empty_set ? 1 : 0);
+ 			bool	all = ((GroupByClause *) stmt-groupClause)-all;
+ 		
+ 			cteedstmt = makeSelectStmt(NIL, NIL);
+ 			cteedstmt-intoClause = stmt-intoClause;
+ 			cteedstmt-sortClause = stmt-sortClause;
+ 			cteedstmt-limitOffset = stmt-limitOffset;
+ 			cteedstmt-limitCount = stmt-limitCount;
+ 			cteedstmt-lockingClause = stmt-lockingClause;
+ 		
+ 			cte-ctename = **g**;
+ 			cte-ctequery = (Node *) stmt;
+ 			cte-location = -1;
+ 		
+ 			cteedstmt-withClause = makeNode(WithClause);
+ 			cteedstmt-withClause-ctes = list_make1(cte);
+ 			cteedstmt-withClause-recursive = false;
+ 			cteedstmt-withClause-location = -1;
+ 		
+ 			/* when is more than one grouping set, then we should generate setop node */
+ 			if (ngroupingsets  1)
+ 			{
+ /* add quuery under union all for every grouping set */
+ SelectStmt	*larg = NULL;
+ SelectStmt	*rarg;
+ ListCell*lc;
+ 			
+ foreach(lc, gss-set_list)
+ {
+ 	List	*groupClause;
+ 
+ 	Assert(IsA(lfirst(lc), List));
+ 	groupClause = (List *) lfirst(lc);
+ 
+ 	if (larg == NULL)
+ 	{
+ 		larg = makeSelectStmt(copyObject(stmt-targetList),
+ 	list_make1(makeRangeVar(NULL, **g**, -1)));
+ 		larg-groupClause = (Node *) groupClause;
+ 		larg-havingClause = copyObject(stmt-havingClause);
+ 	}
+ 	else
+ 	{
+ 		SelectStmt	*setop = makeSelectStmt(NIL, NIL);
+ 	
+ 		rarg = makeSelectStmt(copyObject(stmt-targetList),
+