doc: More comments on GroupedHashAggregateStream refactor

[2010YOUY01]

Which issue does this PR close?

• Closes #.

Rationale for this change

Original discussion: #23165

This PR adds more comments to explain the on-going refactor, to reduce confusion.

• refactor: [EPIC] Split Aggregation Logic into Dedicated Streams #22710

cc @alamb@Rachelint

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

[alamb]

Let's do it

@2010YOUY01 what is the best way to support your work getting this refactoring done?

I think the worst thing that could happen is that we leave the code in a half way migrated state (especially if new features are implemented in the new code)

Perhaps we can make this a "required" (at least target" feature for the 55 release)

[2010YOUY01]

Perhaps we can make this a "required" (at least target" feature for the 55 release)

Thank you! It's doable from my side.

I think the worst thing that could happen is that we leave the code in a half way migrated state (especially if new features are implemented in the new code)

There are several projects that depend on this refactor, specifically

• feat(aggregate): cost-aware partial-aggregation skip (opt-in) #22518 with @zhuqi-lucas
• Improve aggregate performance with adaptive sizing in accumulators / avoiding reallocations in accumulators #7065 with @Rachelint

Perhaps we could get this refactor over the finish line first, to avoid this half refactored state? Anyway we can further coordinate if there are any other potential concurrent patches.

[alamb]

Perhaps we could get this refactor over the finish line first, to avoid this half refactored state? Anyway we can further coordinate if there are any other potential concurrent patches.

Yes, I think focusing on getting the refactor done would be ideal. I think this is a (very) important key technical change on one of the most performance critical parts of DataFusion, and thus I plan to devote significant time to reviewing to assist making it happen