Re: Wrong results with grouping sets

Поиск
Список
Период
Сортировка
От Richard Guo
Тема Re: Wrong results with grouping sets
Дата
Msg-id CAMbWs4_rqRShhmw56Y9vjbhDH3t7avY+d8jX5+D35e=HaH62zg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Wrong results with grouping sets  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Wrong results with grouping sets
Список pgsql-hackers

On Sun, Jan 7, 2024 at 4:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't think this is going in quite the right direction.  We have
many serious problems with grouping sets (latest one today at [1]),
and I don't believe that hacking around EquivalenceClasses is going
to fix them all.

I think that what we really need to do is invent a new kind of RTE
representing the output of the grouping step, with columns that
are the Vars or expressions being grouped on.  Then we would make
the parser actually replace subexpressions in the tlist with Vars
referencing this new RTE (that is, change check_ungrouped_columns
into something that modifies the expression tree into something that
contains no Vars that aren't grouping-RTE Vars).  In this way the
output of the parser directly expresses the semantic requirement that
certain subexpressions be gotten from the grouping output rather than
computed some other way.

The trick is to do this without losing optimization capability.
We could have the planner replace these Vars with the underlying
Vars in cases where it's safe to do so (perhaps after adding a
nullingrel bit that references the grouping RTE).  If a grouping
column is an expression, we might be able to replace the reference
Vars with PHVs as you've done here ... but I think we need the
parser infrastructure fixed first.

Sorry it takes me some time to get back to this thread.

I think you're right.  To fix the cases where there are subqueries in
the grouping sets, as in Geoff's example, it seems that we'll have to
fix the parser infrastructure by inventing a new RTE for the grouping
step and replacing the subquery expressions with Vars referencing this
new RTE, so that there is only one instance of the subquery in the
parser output.

I have experimented with this approach, and here is the outcome.  The
patch fixes Geoff's query, but it's still somewhat messy as I'm not
experienced enough in the parser code.  And the patch has not yet
implemented the nullingrel bit manipulation trick.  Before proceeding
further with this patch, I'd like to know if it is going in the right
direction.

Thanks
Richard
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Peter Eisentraut
Дата:
Сообщение: Re: Minor cleanups in the SSL tests
Следующее
От: Jelte Fennema-Nio
Дата:
Сообщение: Re: First draft of PG 17 release notes