Re: Grouping Sets: Fix unrecognized node type bug

Поиск
Список
Период
Сортировка
От Kyotaro HORIGUCHI
Тема Re: Grouping Sets: Fix unrecognized node type bug
Дата
Msg-id 20150717.171309.187064257.horiguchi.kyotaro@lab.ntt.co.jp
обсуждение исходный текст
Ответ на Re: Grouping Sets: Fix unrecognized node type bug  (Jeevan Chalke <jeevan.chalke@enterprisedb.com>)
Ответы Re: Grouping Sets: Fix unrecognized node type bug  (Andrew Gierth <andrew@tao11.riddles.org.uk>)
Список 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 по дате отправления:

Предыдущее
От: Haribabu Kommi
Дата:
Сообщение: Re: Parallel Seq Scan
Следующее
От: Florent Guiliani
Дата:
Сообщение: Re: Retrieve the snapshot's LSN