You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "cetra3 (via GitHub)" <gi...@apache.org> on 2026/09/30 01:36:07 UTC
[PR] fix: preserve grouping set member names during type coercion [datafusion]
cetra3 opened a new pull request, #25891:
URL: https://github.com/apache/datafusion/pull/25891
## Which issue does this PR close?
- Closes #25890.
## Rationale for this change
`GROUP BY ROLLUP`, `CUBE`, or `GROUPING SETS` fails during analysis when a member is a scalar function with an implicitly coerced literal argument, such as `date_bin('5 minutes', ts)`:
```
type_coercion
caused by
Schema error: No field named "date_bin(Utf8(""5 minutes""),t.ts)". Did you mean '"ROLLUP (date_bin(Utf8(""5 minutes""),t.ts))"'?
```
Type coercion now turns the literal into an interval value, which renames the member. `TypeCoercion` then restored the original name by aliasing the whole `Expr::GroupingSet`. An aliased grouping set is no longer recognized as a grouping set: it becomes one ordinary group key named `ROLLUP (...)`, with no `__grouping_id`, and the projection above can no longer find the member.
## What changes are included in this PR?
`TypeCoercion` now coerces each grouping set member individually through `map_children`, saving and restoring each member's name, and leaves the grouping set itself unaliased. This is the same approach `SimplifyExpressions` has used since #14888.
## What is the testing strategy for this PR?
- New sqllogictest cases in `grouping.slt`: `ROLLUP` over the coerced member with `grouping()` in the select list, and `GROUPING SETS` with a qualified column next to the coerced member, which is repeated across sets. Both fail without this change.
- New unit test `scalar_udf_in_grouping_set` in `type_coercion.rs`, checking the analyzed plan shape and that a second coercion pass leaves the plan unchanged.
## Are there any user-facing changes?
Queries of this shape now plan and run. No public API changes.
## AI assistance
This change was prepared with AI assistance. Assumptions called out for reviewers:
- The fix is scoped to `TypeCoercion`. `SimplifyExpressions` already carries the same grouping set workaround, and other rules that use `NamePreserver` on an `Aggregate` could in principle hit the same whole-grouping-set alias. Moving this into `NamePreserver` would fix it in one place, but changes the public `SavedName` API, so I left that for a possible follow-up.
- Skipping `TypeCoercionRewriter` on the `Expr::GroupingSet` node itself relies on the rewriter passing grouping sets through unchanged, which it does today.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: preserve grouping set member names during type coercion [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #25891:
URL: https://github.com/apache/datafusion/pull/25891
--
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: preserve grouping set member names during type coercion [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25891:
URL: https://github.com/apache/datafusion/pull/25891#issuecomment-5902611299
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25891?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `84.37500%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 82.58%. Comparing base ([`1d9be2e`](https://app.codecov.io/gh/apache/datafusion/commit/1d9be2e10794994fc1495c35363658dc7db0adf7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`80ea66e`](https://app.codecov.io/gh/apache/datafusion/commit/80ea66e3c255833eaaea5748ef47c7f5b2c9e1ad?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25891?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/optimizer/src/analyzer/type\_coercion.rs](https://app.codecov.io/gh/apache/datafusion/pull/25891?src=pr&el=tree&filepath=datafusion%2Foptimizer%2Fsrc%2Fanalyzer%2Ftype_coercion.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9vcHRpbWl6ZXIvc3JjL2FuYWx5emVyL3R5cGVfY29lcmNpb24ucnM=) | 84.37% | [1 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25891?src=pr&el=tree&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 #25891 +/- ##
==========================================
- Coverage 82.58% 82.58% -0.01%
==========================================
Files 1144 1144
Lines 441510 441541 +31
Branches 441510 441541 +31
==========================================
+ Hits 364615 364630 +15
- Misses 54844 54852 +8
- Partials 22051 22059 +8
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25891?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