[Review] Grouping Sets patch

Hi Stehule,

I have looked at the patch and it looks great. Here are my observation

Compilation
----------------
1 - Patch applied successfully on CVS-HEAD
2 - No compilation error found

Code
-------
1 - Style of code is very close to the existing PG code.
2 - Comments look OK to me.
3 - I haven't looked deep into the code. As this is a WIP patch so I
gave my emphasis on testing and verifying the concept.

BTW I have not tested the cases you have mentioned. Here are the some
sample test cases (I haven't paste complete test cases I have used)

CREATE TABLE population_tbl (country varchar, male NUMERIC, female NUMERIC);

INSERT INTO population_tbl values ('Australia',1,100);
INSERT INTO population_tbl values ('Denmark',2,200);
INSERT INTO population_tbl values ('Germany',3,300);
INSERT INTO population_tbl values ('Netherlands',4,400);
INSERT INTO population_tbl values ('United States',5,500);
INSERT INTO population_tbl values ('Pakistan',6,600);

--GROUPING SET
SELECT country,male,female FROM population_tbl GROUP BY GROUPING
SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY GROUPING
SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY
GROUPING SETS(country,(male,female));

SELECT country,male,female FROM population_tbl GROUP BY ALL GROUPING
SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY ALL GROUPING
SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY ALL
GROUPING SETS(country,(male,female));

SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
GROUPING SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
GROUPING SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY
DISTINCT GROUPING SETS(country,(male,female));

SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY GROUPING SETS(country,(male,female));

SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY ALL GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY ALL GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY ALL GROUPING SETS(country,(male,female));

SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY DISTINCT GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY DISTINCT GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY DISTINCT GROUPING SETS(country,(male,female));

SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY GROUPING SETS(country,(male,female));

SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY ALL GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY ALL GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY ALL GROUPING SETS(country,(male,female));

SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY DISTINCT GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY DISTINCT GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY DISTINCT GROUPING SETS(country,(male,female));

--Neg: SELECT country,male,female,sum(male) FROM population_tbl GROUP
BY GROUPING SETS((country),(male),female,());

--ROLLUP
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP((country),(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP((country,male),(female));

SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP((country),(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP((country,male),(female));

SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
ROLLUP(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
ROLLUP(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
ROLLUP(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
ROLLUP((country),(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
ROLLUP((country,male),(female));

--CUBE
SELECT country,male,female FROM population_tbl GROUP BY
CUBE(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY
CUBE(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY
CUBE(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY
CUBE((country),(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY
CUBE((country,male),(female));

SELECT country,male,female FROM population_tbl GROUP BY ALL
CUBE(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY ALL
CUBE(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
CUBE(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
CUBE((country),(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
CUBE((country,male),(female));

SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
CUBE(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
CUBE(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
CUBE(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
CUBE((country),(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
CUBE((country,male),(female));

--Problems
--------

1 - ORDER BY CLAUSE is not working after the patch

--After Patch
----------------

postgres=# SELECT country,male,female FROM population_tbl order by male DESC;
country | male | female
---------------+------+--------
Australia | 1 | 100
Denmark | 2 | 200
Germany | 3 | 300
Netherlands | 4 | 400
United States | 5 | 500
Pakistan | 6 | 600
(6 rows)

--Before patch
-------------------

postgres=# SELECT country,male,female FROM population_tbl order by male DESC;
country | male | female
---------------+------+--------
Pakistan | 6 | 600
United States | 5 | 500
Netherlands | 4 | 400
Germany | 3 | 300
Denmark | 2 | 200
Australia | 1 | 100
(6 rows)

Some Minor code observations
----------------------
1 - IMHO we should use enum instead of #define

i.e
#define CUBE_OP 1
#define ROLLUP_OP 2
#define FUNCCALL_OP 3

--
Ibrar Ahmed
EnterpriseDB http://www.enterprisedb.com