Re: [HACKERS] Refactor to split nodeAgg.c?

[David Rowley]

On 30 June 2015 at 14:33, Jeff Davis  wrote:

> I was going to rebase my HashAgg patch, and got some conflicts related
> to the grouping sets patch. I could probably sort them out, but I think
> that may be the tipping point where we want to break up nodeAgg.c into
> nodeSortedAgg.c and nodeHashAgg.c, and probably a common file as well.
>
> This would also (I hope) be convenient for Simon and David Rowley, who
> have been hacking on aggregates in general.
>
>
That would be more inconvenient right now as I have a pending patch which
makes quite a number of changes which are all over nodeAgg.c.
https://commitfest.postgresql.org/5/271/

Regards

David Rowley

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] Refactor to split nodeAgg.c?

[Andres Freund]

Hi,

On 2015-06-29 19:33:58 -0700, Jeff Davis wrote:
> I was going to rebase my HashAgg patch, and got some conflicts related
> to the grouping sets patch. I could probably sort them out, but I think
> that may be the tipping point where we want to break up nodeAgg.c into
> nodeSortedAgg.c and nodeHashAgg.c, and probably a common file as well.

I'm not sure that's going to be helpful and clean without a significant
amount of duplication. Grouping sets right now use sorting, but Andrew
Gierth already is working on a patch that employs hashing for individual
group of groups that support it and where the aggregated state is deemed
small enough.  That implies a fair amount of coupling between the sorted
and hashed aggregation modes.

I'm not sure that conflicts due to GS can be taken as an argument to
split the file - I doubt there'd be significantly fewer with a splitup
since common datastructures have been changed.

That said, I think e.g. splitting out the lowest level of interaction
with aggregation functions and transition layers could be split off
without too much pain.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Refactor to split nodeAgg.c?

[Tom Lane]

Jeff Davis  writes:
> I was going to rebase my HashAgg patch, and got some conflicts related
> to the grouping sets patch. I could probably sort them out, but I think
> that may be the tipping point where we want to break up nodeAgg.c into
> nodeSortedAgg.c and nodeHashAgg.c, and probably a common file as well.

> This would also (I hope) be convenient for Simon and David Rowley, who
> have been hacking on aggregates in general.

> Anyone see a reason I shouldn't give this a try?

As with the discussion about pgbench, it's hard to opine about this
without seeing a concrete refactoring proposal.  But if you want to
try, now, very early in the dev cycle, would be the best time to try.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Refactor to split nodeAgg.c?

[Jeff Davis]

I was going to rebase my HashAgg patch, and got some conflicts related
to the grouping sets patch. I could probably sort them out, but I think
that may be the tipping point where we want to break up nodeAgg.c into
nodeSortedAgg.c and nodeHashAgg.c, and probably a common file as well.

This would also (I hope) be convenient for Simon and David Rowley, who
have been hacking on aggregates in general.

Anyone see a reason I shouldn't give this a try?

Regards,
Jeff Davis




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers