>>>>> "Kyotaro" == Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> writes:
Kyotaro> Hello, this looks to be a kind of thinko. The attached patch
Kyotaro> fixes it.
No, that's still wrong. Just knowing that there is a List is not enough
to tell whether to concat it or append it.
Jeevan's original patch tries to get around this by making the RowExpr
case wrap another List around its result (which is then removed by the
concat), but this is the wrong approach too because it breaks nested
RowExprs (which isn't valid syntax in the spec, because the spec allows
only column references in GROUP BY, not arbitrary expressions, but which
we have no reason not to support).
Attached is the current version of my fix (with Jeevan's regression
tests plus one of mine).
--
Andrew (irc:RhodiumToad)
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index e90e1d6..5a48a02 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -1734,7 +1734,7 @@ findTargetlistEntrySQL99(ParseState *pstate, Node *node, List **tlist,
* Inside a grouping set (ROLLUP, CUBE, or GROUPING SETS), we expect the
* content to be nested no more than 2 deep: i.e. ROLLUP((a,b),(c,d)) is
* ok, but ROLLUP((a,(b,c)),d) is flattened to ((a,b,c),d), which we then
- * normalize to ((a,b,c),(d)).
+ * (later) normalize to ((a,b,c),(d)).
*
* CUBE or ROLLUP can be nested inside GROUPING SETS (but not the reverse),
* and we leave that alone if we find it. But if we see GROUPING SETS inside
@@ -1803,9 +1803,16 @@ flatten_grouping_sets(Node *expr, bool toplevel, bool *hasGroupingSets)
foreach(l2, gset->content)
{
- Node *n2 = flatten_grouping_sets(lfirst(l2), false, NULL);
+ Node *n1 = lfirst(l2);
+ Node *n2 = flatten_grouping_sets(n1, false, NULL);
- result_set = lappend(result_set, n2);
+ if (IsA(n1, GroupingSet) &&
+ ((GroupingSet *)n1)->kind == GROUPING_SET_SETS)
+ {
+ result_set = list_concat(result_set, (List *) n2);
+ }
+ else
+ result_set = lappend(result_set, n2);
}
/*
diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out
index 842c2ae..ff3ba9b 100644
--- a/src/test/regress/expected/groupingsets.out
+++ b/src/test/regress/expected/groupingsets.out
@@ -145,6 +145,127 @@ select a, b, sum(c), sum(sum(c)) over (order by a,b) as rsum
| | 12 | 36
(6 rows)
+-- nesting with grouping sets
+select sum(c) from gstest2
+ group by grouping sets((), grouping sets((), grouping sets(())))
+ order by 1 desc;
+ sum
+-----
+ 12
+ 12
+ 12
+(3 rows)
+
+select sum(c) from gstest2
+ group by grouping sets((), grouping sets((), grouping sets(((a, b)))))
+ order by 1 desc;
+ sum
+-----
+ 12
+ 12
+ 8
+ 2
+ 2
+(5 rows)
+
+select sum(c) from gstest2
+ group by grouping sets(grouping sets(rollup(c), grouping sets(cube(c))))
+ order by 1 desc;
+ sum
+-----
+ 12
+ 12
+ 6
+ 6
+ 6
+ 6
+(6 rows)
+
+select sum(c) from gstest2
+ group by grouping sets(a, grouping sets(a, cube(b)))
+ order by 1 desc;
+ sum
+-----
+ 12
+ 10
+ 10
+ 8
+ 4
+ 2
+ 2
+(7 rows)
+
+select sum(c) from gstest2
+ group by grouping sets(grouping sets((a, (b))))
+ order by 1 desc;
+ sum
+-----
+ 8
+ 2
+ 2
+(3 rows)
+
+select sum(c) from gstest2
+ group by grouping sets(grouping sets((a, b)))
+ order by 1 desc;
+ sum
+-----
+ 8
+ 2
+ 2
+(3 rows)
+
+select sum(c) from gstest2
+ group by grouping sets(grouping sets(a, grouping sets(a), a))
+ order by 1 desc;
+ sum
+-----
+ 10
+ 10
+ 10
+ 2
+ 2
+ 2
+(6 rows)
+
+select sum(c) from gstest2
+ group by grouping sets(grouping sets(a, grouping sets(a, grouping sets(a), ((a)), a, grouping sets(a), (a)), a))
+ order by 1 desc;
+ sum
+-----
+ 10
+ 10
+ 10
+ 10
+ 10
+ 10
+ 10
+ 10
+ 2
+ 2
+ 2
+ 2
+ 2
+ 2
+ 2
+ 2
+(16 rows)
+
+select sum(c) from gstest2
+ group by grouping sets((a,(a,b)), grouping sets((a,(a,b)),a))
+ order by 1 desc;
+ sum
+-----
+ 10
+ 8
+ 8
+ 2
+ 2
+ 2
+ 2
+ 2
+(8 rows)
+
-- empty input: first is 0 rows, second 1, third 3 etc.
select a, b, sum(v), count(*) from gstest_empty group by grouping sets ((a,b),a);
a | b | sum | count
diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql
index 0bffb85..d886fae 100644
--- a/src/test/regress/sql/groupingsets.sql
+++ b/src/test/regress/sql/groupingsets.sql
@@ -73,6 +73,35 @@ select grouping(a), a, array_agg(b),
select a, b, sum(c), sum(sum(c)) over (order by a,b) as rsum
from gstest2 group by rollup (a,b) order by rsum, a, b;
+-- nesting with grouping sets
+select sum(c) from gstest2
+ group by grouping sets((), grouping sets((), grouping sets(())))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets((), grouping sets((), grouping sets(((a, b)))))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets(grouping sets(rollup(c), grouping sets(cube(c))))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets(a, grouping sets(a, cube(b)))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets(grouping sets((a, (b))))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets(grouping sets((a, b)))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets(grouping sets(a, grouping sets(a), a))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets(grouping sets(a, grouping sets(a, grouping sets(a), ((a)), a, grouping sets(a), (a)), a))
+ order by 1 desc;
+select sum(c) from gstest2
+ group by grouping sets((a,(a,b)), grouping sets((a,(a,b)),a))
+ order by 1 desc;
+
-- empty input: first is 0 rows, second 1, third 3 etc.
select a, b, sum(v), count(*) from gstest_empty group by grouping sets ((a,b),a);
select a, b, sum(v), count(*) from gstest_empty group by grouping sets ((a,b),());
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
