Describe the bug
AggregateFunction.phase is never read: the string phase does not appear anywhere under datafusion/substrait/src/logical_plan/consumer/. A measure declared INITIAL_TO_INTERMEDIATE is planned as an ordinary aggregate, and the plan is accepted with no error.
Measured on 3266eaa91. The plans read a named table t_avg(c0 i64 NOT NULL) holding rows 1 and 2, and carry one avg measure over c0:
| plan |
phase |
declared output |
DataFusion returns |
PlanRel.rel |
INITIAL_TO_INTERMEDIATE |
STRUCT<i64,i64> |
avg(t_avg.c0):Float64?, 1.5 |
PlanRel.rel |
INITIAL_TO_RESULT |
i64? |
avg(t_avg.c0):Float64?, 1.5 |
PlanRel.root |
INITIAL_TO_INTERMEDIATE |
STRUCT<i64,i64> |
rejected: Names list must match exactly to nested schema, but found 1 uses for 3 names |
Two different phases, one answer — and that answer is neither declared type. functions_arithmetic.yaml gives avg:i64 return: i64? and says it truncates for integral types, so 1.5 is DataFusion's own avg: not the intermediate struct the first plan asks for, and not the final value the second one declares.
The third row is why this stays out of sight. A struct-returning measure needs three names depth-first in RelRoot — the column, then the struct's two fields — and that plan is rejected on names, so anyone writing the rooted form reads an error about something else.
Expected behavior
Producing the intermediate state does not look available here: LogicalPlan::Aggregate carries no phase, and partial/final is the physical planner's business. Rejecting what cannot be honoured may be the whole of the fix, but the boundary matters. phase defaults to 0 (UNSPECIFIED), and that is what DataFusion's own producer writes (producer/expr/aggregate_function.rs:68), so a rule has to accept 0 alongside INITIAL_TO_RESULT and refuse the rest. Worth knowing before you pick the rule: algebra.proto in spec v0.102.0 comments that constant // Implies INTERMEDIATE_TO_RESULT, so reading 0 by its documented meaning would reject the plans your own producer emits. That contradiction is the spec's, not yours.
Window functions carry the same field. I have not tested them.
To Reproduce
The three plans are at probe/phase-cases — protobuf-JSON with the .bin alongside, plus the one-column table registered above. Glad to open a PR adding them as consumer tests if that is where you would want them.
Describe the bug
AggregateFunction.phaseis never read: the stringphasedoes not appear anywhere underdatafusion/substrait/src/logical_plan/consumer/. A measure declaredINITIAL_TO_INTERMEDIATEis planned as an ordinary aggregate, and the plan is accepted with no error.Measured on
3266eaa91. The plans read a named tablet_avg(c0 i64 NOT NULL)holding rows1and2, and carry oneavgmeasure overc0:PlanRel.relINITIAL_TO_INTERMEDIATESTRUCT<i64,i64>avg(t_avg.c0):Float64?,1.5PlanRel.relINITIAL_TO_RESULTi64?avg(t_avg.c0):Float64?,1.5PlanRel.rootINITIAL_TO_INTERMEDIATESTRUCT<i64,i64>Names list must match exactly to nested schema, but found 1 uses for 3 namesTwo different phases, one answer — and that answer is neither declared type.
functions_arithmetic.yamlgivesavg:i64return: i64?and says it truncates for integral types, so1.5is DataFusion's ownavg: not the intermediate struct the first plan asks for, and not the final value the second one declares.The third row is why this stays out of sight. A struct-returning measure needs three names depth-first in
RelRoot— the column, then the struct's two fields — and that plan is rejected on names, so anyone writing the rooted form reads an error about something else.Expected behavior
Producing the intermediate state does not look available here:
LogicalPlan::Aggregatecarries no phase, and partial/final is the physical planner's business. Rejecting what cannot be honoured may be the whole of the fix, but the boundary matters.phasedefaults to 0 (UNSPECIFIED), and that is what DataFusion's own producer writes (producer/expr/aggregate_function.rs:68), so a rule has to accept 0 alongsideINITIAL_TO_RESULTand refuse the rest. Worth knowing before you pick the rule:algebra.protoin spec v0.102.0 comments that constant// Implies INTERMEDIATE_TO_RESULT, so reading 0 by its documented meaning would reject the plans your own producer emits. That contradiction is the spec's, not yours.Window functions carry the same field. I have not tested them.
To Reproduce
The three plans are at
probe/phase-cases— protobuf-JSON with the.binalongside, plus the one-column table registered above. Glad to open a PR adding them as consumer tests if that is where you would want them.