feat(physical-plan): `GroupColumn` support for `List` / `LargeList` by mzabaluev · Pull Request #23648 · apache/datafusion

mzabaluev · 2026-07-16T18:10:24Z

Which issue does this PR close?

Part of feat: add GroupColumn support for List / Struct / Map / FixedSizeList in multi-column GROUP BY #22682.

Rationale for this change

This adds column-native GroupColumn implementations for List<T> and LargeList<T>, so a GROUP BY containing these (along with other supported data types) no longer falls back from GroupValuesColumn to GroupValuesRows.

What changes are included in this PR?

Cherry-picked @zhuqi-lucas's changes in #22706 that pertained to List / LargeList, and made some optimizations.

Are these changes tested?

Cherry-picked the unit tests for the list module from #22706.
Added supported/unsupported cases for List in the group_column_supported_type_matches_make_group_column test.

Benchmarked in a proprietary application using several HashAggregate operations on 20 columns including two string lists, to about 8% cumulative improvement.

Are there any user-facing changes?

No.

This is a minimal cherry-pick of the changes in apache#22706, adding GroupColumn support specifically for List and LargeList data types. Co-Authored-By: Qi Zhu <821684824@qq.com>

Saves allocation of a new array.

Sign in to view

+ for j in 0..lhs_len {
+ if !self.child.equal_to(lhs_start + j, &rhs_sublist, j) {
+ return false;
+ }
+ }


Sign in to view

+ for j in 0..n {
+ self.child.append_val(&sublist, j)?;
+ }


codecov-commenter · 2026-07-16T18:36:33Z

Codecov Report

❌ Patch coverage is 96.91630% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.01%. Comparing base (4b48cb0) to head (a08c98e).
⚠️ Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
...src/aggregates/group_values/multi_group_by/list.rs	98.09%	6 Missing and 5 partials ⚠️
.../src/aggregates/group_values/multi_group_by/mod.rs	90.38%	4 Missing and 6 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #23648 +/- ##
==========================================
- Coverage 81.06% 81.01% -0.05% 
==========================================
Files 1106 1107 +1 Lines 382252 383838 +1586 Branches 382252 383838 +1586 ==========================================
+ Hits 309861 310980 +1119 - Misses 54083 54543 +460 - Partials 18308 18315 +7

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

KonaeAkira · 2026-07-16T21:17:22Z

+ DataType::List(child_field) => {
+ let child = make_group_column(child_field.as_ref())?;
+ v.push(Box::new(list::ListGroupValueBuilder::<i32>::new(
+ Arc::clone(child_field),
+ child,
+ )));
+ }


I'm not sure about using ListGroupValueBuilder::<i32>. What happens when the concatenated length of all the children exceeds i32::MAX? I think ListGroupValueBuilder should drop the O: OffsetSizeTrait generic and always use usize to store its offsets.

As I understand it, the build/take_n of this builder must produce a ListArray, with i32 offsets. So an overflow would result in an error.

Co-authored-by: Trương Hoàng Long <longtruong2411@gmail.com>

mzabaluev · 2026-07-17T10:15:35Z

- // First-n offsets: 0, off[1], ..., off[n].
- let first_n_offsets: Vec<O> = self.offsets[..=n].to_vec();
-
- // Remaining offsets shifted so that what was offsets[n] becomes 0.
- // Overwrite the array in place.
- // SAFETY: the write range is at most as large as offsets.len().
- // Values in the possible overlap are read before being overwritten.
- unsafe {
- let dst = self.offsets.as_mut_ptr();
- for (i, &off) in self.offsets[n..].iter().enumerate() {
- *dst.add(i) = off - cut_offset;
- }
- }
- self.offsets.truncate(self.offsets.len() - n);


@KonaeAkira if we're going into micro-benchmarking territory, the previous code might be better in smaller take case when the remaining vector is drained in place, because it's subtract-while-copying vs. copy, then subtract in place. But this highly depends on vectorization, and the other case benefits from a smaller alloc-and-copy. Maybe there needs to be another split helper with a map closure to address both cases.

One more thing that just occurred to me: split_vec_min_alloc(&mut self.offsets, n) returns a vector with exact capacity in case n * 2 <= self.offsets.len():
datafusion/datafusion/common/src/utils/mod.rs
Lines 405 to 412 in fb8fe7a
pubfnsplit_vec_min_alloc<T>(vec:&mutVec<T>,n:usize) -> Vec<T>{
if n *2 <= vec.len(){
vec.drain(0..n).collect()
}else{
let remaining = vec.split_off(n);
std::mem::replace(vec, remaining)
}
}
in which case first_n_offsets.push(cut_offset); will always reallocate, which is bad.
Maybe a separate PR can address this. This affects bytes.rs as well.

Followed up with #23730.

KonaeAkira

LGTM

Post-merge adaptations: Float16 nested type now has `GroupColumn` support. Use RunEndEncoded as the unsupported leaf type case.

MSRV doesn't let us use `if let`, but instead the whole function can be rewritten to not use a vector to collect the single element (which was asserted to always be the case) and DRY on the error path.

zhuqi-lucas · 2026-08-10T13:10:51Z

We can create a benchmark PR target this PR first, so we can compare it in CI benchmark.

mzabaluev · 2026-08-10T13:29:32Z

We can create a benchmark PR target this PR first, so we can compare it in CI benchmark.

Sure, go ahead.

mzabaluevand others added 3 commits July 16, 2026 14:32

feat(physical-plan): GroupColumn support for lists
c2d9411
This is a minimal cherry-pick of the changes in apache#22706, adding GroupColumn support specifically for List and LargeList data types. Co-Authored-By: Qi Zhu <821684824@qq.com>

perf: shift offsets array in place
831419b
Saves allocation of a new array.

perf: downcast array once in vectorized methods
cf66d5e

github-actionsBot added the physical-plan Changes to the physical-plan crate label Jul 16, 2026

mzabaluev mentioned this pull request Jul 16, 2026
feat: add GroupColumn support for List / Struct / Map / FixedSizeList in multi-column GROUP BY #22682
Open

mzabaluev commented Jul 16, 2026
View reviewed changes

chore: rustfmt
3d477af

KonaeAkira suggested changes Jul 16, 2026
View reviewed changes

mzabaluev requested a review from KonaeAkira July 17, 2026 00:03

KonaeAkira suggested changes Jul 17, 2026
View reviewed changes

refactor: suggestions from code review
553bd2e
Co-authored-by: Trương Hoàng Long <longtruong2411@gmail.com>

mzabaluev-flarion force-pushed the nested-group-column-for-lists branch from 7ee5048 to 553bd2eCompare July 17, 2026 09:52

mzabaluev commented Jul 17, 2026
View reviewed changes

chore: unused mut
ab84d14

KonaeAkira approved these changes Jul 17, 2026
View reviewed changes

chore: use as_generic_list_array
1ae3235

mzabaluev mentioned this pull request Jul 20, 2026
perf: improve splitting of the offset vector in GroupColumn::take_n implementations, adding a new helper fn #23730
Open

mzabaluev added 2 commits August 4, 2026 12:56

Merge branch 'main' into nested-group-column-for-lists
6d6a443

test(physical-plan): fix multi_group_by tests
a660baa
Post-merge adaptations: Float16 nested type now has `GroupColumn` support. Use RunEndEncoded as the unsupported leaf type case.

zhuqi-lucas mentioned this pull request Aug 8, 2026
EPIC: complete GroupValuesColumn type coverage (nested types + remaining primitives) #22715
Closed
7 tasks

Merge branch 'main' into nested-group-column-for-lists
f5289d2

github-actionsBot added auto detected api change Auto detected API change and removed auto detected api change Auto detected API change labels Aug 8, 2026

refactor: rewrite make_group_column
a08c98e
MSRV doesn't let us use `if let`, but instead the whole function can be rewritten to not use a vector to collect the single element (which was asserted to always be the case) and DRY on the error path.

mzabaluev added 2 commits August 10, 2026 16:45

refactor: dedupe code in make_group_column further
5813e6c

Merge branch 'main' into nested-group-column-for-lists
e5c0fd4

Navigation Menu

Uh oh!

feat(physical-plan): `GroupColumn` support for `List` / `LargeList` - #23648

feat(physical-plan): `GroupColumn` support for `List` / `LargeList`#23648
mzabaluev wants to merge 13 commits into
apache:mainfrom
mzabaluev:nested-group-column-for-lists

mzabaluev commented Jul 16, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

codecov-commenter commented Jul 16, 2026•
edited
Loading

Uh oh!

KonaeAkiraJul 16, 2026

Uh oh!

mzabaluevJul 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mzabaluevJul 17, 2026•
edited
Loading

Uh oh!

KonaeAkiraJul 17, 2026•
edited
Loading

Uh oh!

mzabaluevJul 20, 2026

Uh oh!

KonaeAkira left a comment

Uh oh!

zhuqi-lucas commented Aug 10, 2026

Uh oh!

mzabaluev commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	pubfnsplit_vec_min_alloc<T>(vec:&mutVec<T>,n:usize) -> Vec<T>{
	if n *2 <= vec.len(){
	vec.drain(0..n).collect()
	}else{
	let remaining = vec.split_off(n);
	std::mem::replace(vec, remaining)
	}
	}

Uh oh!

Conversation

mzabaluev commented Jul 16, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

codecov-commenter commented Jul 16, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

KonaeAkiraJul 16, 2026

Choose a reason for hiding this comment

Uh oh!

mzabaluevJul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mzabaluevJul 17, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KonaeAkiraJul 17, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzabaluevJul 20, 2026

Choose a reason for hiding this comment

Uh oh!

KonaeAkira left a comment

Choose a reason for hiding this comment

Uh oh!

zhuqi-lucas commented Aug 10, 2026

Uh oh!

mzabaluev commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jul 16, 2026•
edited
Loading

mzabaluevJul 17, 2026•
edited
Loading

KonaeAkiraJul 17, 2026•
edited
Loading