Summary
SubstraitToCalcite does not reliably preserve the
AggregateFunctionInvocation.outputType carried by a Substrait plan. It initially puts the
declared type on the AggregateCall, but
RelBuilder.aggregate(groupKey, aggregateCalls) rebuilds the calls and lets Calcite infer their
return types again.
As a result, a self-describing Substrait plan can change aggregate width and/or nullability during
Substrait → Calcite conversion.
Reproduced on released io.substrait:isthmus:0.88.0 and on main at 48133f9.
Minimal reproduction
ConverterProvider provider = new ConverterProvider();
SubstraitBuilder b = new SubstraitBuilder(provider.getExtensions());
TypeCreator R = TypeCreator.of(false);
TypeCreator N = TypeCreator.of(true);
Rel input =
b.namedScan(
List.of("example"),
List.of("i", "f", "group_key"),
List.of(R.I32, R.FP32, N.STRING));
Rel aggregate =
b.aggregate(
rel -> b.grouping(rel, 2),
rel -> List.of(
withOutputType(b.sum(rel, 0), R.I64),
withOutputType(b.avg(rel, 1), R.FP32)),
input);
RelNode converted = new SubstraitToCalcite(provider).convert(aggregate);
Expected output types:
SUM: required i64 (BIGINT NOT NULL)
AVG: required fp32 (REAL NOT NULL)
Current behavior:
SUM becomes nullable INTEGER, re-derived from its i32 argument
AVG keeps the base REAL type but loses the declared required nullability
The first mismatch is observed as:
expected: BIGINT
but was: INTEGER
Why this is distinct from #336
#336 discusses which nullability an aggregate should be declared with when Substrait and Calcite
inference differ. This issue is narrower: once a concrete outputType is already present in the
plan, the transport conversion should preserve it exactly rather than replace it with another
inference policy.
Consumers can then apply their own compatibility/subtyping policy. The converter should not
silently alter the declaration.
This is also related to the return-type work planned in #1012, but it is a standalone round-trip
correctness defect and can be fixed independently.
Matching compatibility is a separate consumer policy
This issue does not propose making the declared output type part of SqlOperator.equals.
Whether, for example, a wider or non-nullable aggregate can satisfy another plan is a consumer's
matching/subtyping policy rather than a conversion rule.
If the implementation carries the declaration in a private operator wrapper, consider exposing it
through a small public helper such as:
Optional<RelDataType> AggregateFunctions.declaredOutputType(SqlAggFunction function)
That would be symmetric with AggregateFunctions.withoutDeclaredOutputType(...) and would let
consumers implement an explicit policy without depending on a private wrapper. Consumers operating
on an AggregateCall can also read call.getType().
Acceptance criteria
- Substrait → Calcite preserves the exact declared aggregate output type, including width,
precision/scale, and nullability.
- At minimum, regression coverage includes SUM width + nullability and AVG nullability.
- Substrait → Calcite → Substrait preserves those declared types.
- Calcite → Substrait aggregate function lookup still works for any per-invocation
wrapper/operator used to carry the declared type.
- Operator equality remains type-agnostic unless a consumer explicitly adds its own compatibility
policy.
A working patch and regression test are ready locally.
Summary
SubstraitToCalcitedoes not reliably preserve theAggregateFunctionInvocation.outputTypecarried by a Substrait plan. It initially puts thedeclared type on the
AggregateCall, butRelBuilder.aggregate(groupKey, aggregateCalls)rebuilds the calls and lets Calcite infer theirreturn types again.
As a result, a self-describing Substrait plan can change aggregate width and/or nullability during
Substrait → Calcite conversion.
Reproduced on released
io.substrait:isthmus:0.88.0and onmainat48133f9.Minimal reproduction
Expected output types:
SUM: requiredi64(BIGINT NOT NULL)AVG: requiredfp32(REAL NOT NULL)Current behavior:
SUMbecomes nullableINTEGER, re-derived from itsi32argumentAVGkeeps the baseREALtype but loses the declared required nullabilityThe first mismatch is observed as:
Why this is distinct from #336
#336 discusses which nullability an aggregate should be declared with when Substrait and Calcite
inference differ. This issue is narrower: once a concrete
outputTypeis already present in theplan, the transport conversion should preserve it exactly rather than replace it with another
inference policy.
Consumers can then apply their own compatibility/subtyping policy. The converter should not
silently alter the declaration.
This is also related to the return-type work planned in #1012, but it is a standalone round-trip
correctness defect and can be fixed independently.
Matching compatibility is a separate consumer policy
This issue does not propose making the declared output type part of
SqlOperator.equals.Whether, for example, a wider or non-nullable aggregate can satisfy another plan is a consumer's
matching/subtyping policy rather than a conversion rule.
If the implementation carries the declaration in a private operator wrapper, consider exposing it
through a small public helper such as:
That would be symmetric with
AggregateFunctions.withoutDeclaredOutputType(...)and would letconsumers implement an explicit policy without depending on a private wrapper. Consumers operating
on an
AggregateCallcan also readcall.getType().Acceptance criteria
precision/scale, and nullability.
wrapper/operator used to carry the declared type.
policy.
A working patch and regression test are ready locally.