You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "fornwall (via GitHub)" <gi...@apache.org> on 2026/09/13 23:50:08 UTC
[PR] fix: support zero-field struct group keys in aggregate emit [datafusion]
fornwall opened a new pull request, #25282:
URL: https://github.com/apache/datafusion/pull/25282
## Which issue does this PR close?
Closes #25281.
## Rationale for this change
`GROUP BY` and `SELECT DISTINCT` over a group key whose type contains a zero-field struct (`Struct()`) panicked when the aggregate emitted:
```sql
select arrow_cast(null, 'Struct()') as s, count(*) from (values (1),(2),(3)) group by 1;
-- thread 'tokio-rt-worker' panicked ... dictionary re-encode during emit:
-- InvalidArgumentError("use StructArray::try_new_with_length or StructArray::new_empty_fields ...")
```
The same panic hit `SELECT DISTINCT` and keys where the empty struct is nested inside another struct. A zero-field struct is a valid Arrow type and should group like any other key.
## What changes are included in this PR?
`encode_array_if_necessary` in `group_values/row.rs` rebuilt every struct column with `StructArray::try_new`, which arrow rejects for zero fields since there is no child column to derive the length from. It now uses `StructArray::try_new_with_length` with the source array's length, so zero-field structs are rebuilt like any other struct. This function is shared by `GroupValuesRows` and the row-backed `GroupColumn`, so both paths are covered.
## What is the testing strategy for this PR?
Three new `sqllogictest` cases at the end of `struct.slt` (whole-key, `DISTINCT`, and nested inside `named_struct`). All three panicked before the fix and pass after it.
## Are there any user-facing changes?
No, other than the panic being fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: support zero-field struct group keys in aggregate emit [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on PR #25282:
URL: https://github.com/apache/datafusion/pull/25282#issuecomment-5951930877
Thanks @fornwall @kosiew
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: support zero-field struct group keys in aggregate emit [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on PR #25282:
URL: https://github.com/apache/datafusion/pull/25282#issuecomment-5933347090
re-trigger CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: support zero-field struct group keys in aggregate emit [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25282:
URL: https://github.com/apache/datafusion/pull/25282#issuecomment-5657292140
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25282?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 81.90%. Comparing base ([`15f32dd`](https://app.codecov.io/gh/apache/datafusion/commit/15f32dd7ac751952fffc53906144dfa67c5e868f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f2dd806`](https://app.codecov.io/gh/apache/datafusion/commit/f2dd8060b19344e37fde97d7b7322251b36ad7ed?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25282 +/- ##
==========================================
- Coverage 81.90% 81.90% -0.01%
==========================================
Files 1134 1134
Lines 425200 425201 +1
Branches 425200 425201 +1
==========================================
- Hits 348268 348251 -17
- Misses 56291 56304 +13
- Partials 20641 20646 +5
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25282?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: support zero-field struct group keys in aggregate emit [datafusion]
Posted by "fornwall (via GitHub)" <gi...@apache.org>.
fornwall commented on PR #25282:
URL: https://github.com/apache/datafusion/pull/25282#issuecomment-5949311457
> can you resolve the merge conflicts?
@kosiew Did that now, rebased and pushed!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: support zero-field struct group keys in aggregate emit [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #25282:
URL: https://github.com/apache/datafusion/pull/25282#issuecomment-5947694397
@fornwall
can you resolve the merge conflicts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: support zero-field struct group keys in aggregate emit [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 merged PR #25282:
URL: https://github.com/apache/datafusion/pull/25282
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org