Re: Grouping Sets: Fix unrecognized node type bug - Mailing list pgsql-hackers
| From | Kyotaro HORIGUCHI |
|---|---|
| Subject | Re: Grouping Sets: Fix unrecognized node type bug |
| Date | |
| Msg-id | 20150717.171309.187064257.horiguchi.kyotaro@lab.ntt.co.jp Whole thread Raw |
| In response to | Re: Grouping Sets: Fix unrecognized node type bug (Jeevan Chalke <jeevan.chalke@enterprisedb.com>) |
| Responses |
Re: Grouping Sets: Fix unrecognized node type bug
|
| List | pgsql-hackers |
Hello, this looks to be a kind of thinko. The attached patch
fixes it.
===
According to the comment of transformGroupingSet, it assumes that
the given GROUPING SETS node is already flatted out and
flatten_grouping_sets() does that. The details of the
transformation is described in the comment for the function.
The problmen is what does the function for nested grouping sets.
> Node *n2 = flatten_grouping_sets(lfirst(l2), false, NULL);
> result_set = lappend(result_set, n2);
This does not flattens the list as required. n2 should be
concatenated if it is a list. The attached small patch fixes it
and the problematic query returns sane (perhaps) result.
# Though I don't know the exact definition of the syntax..
=# select sum(c) from gstest2 group by grouping sets ((), grouping sets ((), grouping sets (())));sum
----- 12 12 12
(3 rows)
=# select sum(c) from gstest2 group by grouping sets ((a), grouping sets ((b), grouping sets ((c))));sum
----- 10 2 6 6 8 4
(6 rows)
regards,
At Fri, 17 Jul 2015 11:37:26 +0530, Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote in
<CAM2+6=XPRgumbqWTsczbECC3xjv4zH1ryQ3fwds5uajON1iq2A@mail.gmail.com>
> > Jeevan> It looks like we do support nested GROUPING SETS, I mean Sets
> > Jeevan> withing Sets, not other types. However this nesting is broken.
> >
> > Good catch, but I'm not yet sure your fix is correct; I'll need to look
> > into that.
> >
>
> Sure. Thanks.
>
> However I wonder why we are supporting GROUPING SETS inside GROUPING SETS.
> On Oracle, it is throwing an error.
> We are not trying to be Oracle compatible, but just curious to know.
>
> I have tried restricting it in attached patch.
>
> But it may require few comment adjustment.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index e90e1d6..708ebc9 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -1804,8 +1804,10 @@ flatten_grouping_sets(Node *expr, bool toplevel, bool *hasGroupingSets)
foreach(l2,gset->content) { Node *n2 = flatten_grouping_sets(lfirst(l2), false,
NULL);
-
- result_set = lappend(result_set, n2);
+ if (IsA(n2, List))
+ result_set = list_concat(result_set, (List *)n2);
+ else
+ result_set = lappend(result_set, n2); } /*
pgsql-hackers by date: