Repository navigation
Commit ed2cc50
authored
fix: Set Substrait output_type on aggregate functions (#25090)
## Which issue does this PR close?
Closes #25049.
## Rationale for this change
The Substrait producer exported aggregate calls with `output_type:
None`. Per spec,
`output_type` should carry the function's return type — this mirrors the
fix already
done for scalar functions in #20597.
Verified against substrait-java 0.103.0: it rejects an aggregate measure
with no
`output_type` (`UnsupportedOperationException: Type is not set`) and
accepts the same
plan once this field is set, recovering the correct type.
## What changes are included in this PR?
`from_aggregate_function` now derives the output field via
`Expr::AggregateFunction(..).to_field(schema)` and sets it with
`to_substrait_type_from_field`, the same path scalar functions already
use.
## What is the testing strategy for this PR?
Added `aggregate_function_output_type` unit test covering `count`,
`sum`, `avg`, `min`.
Fails on `main`, passes with this change. Existing
`datafusion-substrait` suite passes
unchanged.
## Are there any user-facing changes?
Substrait plans now declare `output_type` on aggregate calls. No public
API changes.1 parent 1871883 commit ed2cc50
1 file changed
Lines changed: 53 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
| 18 | + | |
19 | 19 | | |
20 | | - | |
21 | 20 | | |
| 21 | + | |
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| 57 | + | |
| 58 | + | |
57 | 59 | | |
58 | 60 | | |
59 | 61 | | |
60 | 62 | | |
61 | 63 | | |
62 | 64 | | |
63 | | - | |
| 65 | + | |
64 | 66 | | |
65 | 67 | | |
66 | 68 | | |
| |||
93 | 95 | | |
94 | 96 | | |
95 | 97 | | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
0 commit comments