Skip to content

Substrait producer omits the required output_type on aggregate functions #25049

Closed

Description

@alexandrefimov

Describe the bug

The Substrait producer emits aggregate calls with output_type: None. The logical plan already has a known result type, but the exported call does not carry it. This affects count, sum, avg and min in the reproducer below.

Reproduced on main at 8a9228164fddeaa03c9ee510b641990e403f2eb5.

To reproduce

From a DataFusion checkout, using its pinned Rust toolchain:

git clone https://github.com/alexandrefimov/substrait-conformance-cases conformance-cases
git -C conformance-cases checkout c4d3ffd74842aa38079b3d523913199dc615a352
mkdir -p datafusion/substrait/examples
cp conformance-cases/probe/datafusion_producer_probe.rs datafusion/substrait/examples/corpus_producer.rs
cargo run --locked -p datafusion-substrait --example corpus_producer -- --aggregate-output-types

Use an unused example filename if corpus_producer.rs already exists. The probe registers an empty named table with a required Int64 column and calls to_substrait_plan for each aggregate. It inspects the produced protobuf directly, without converting it back through a consumer.

Query Logical output type Exported AggregateFunction.output_type
SELECT count(i) FROM t Int64 None
SELECT sum(i) FROM t Int64 None
SELECT avg(i) FROM t Float64 None
SELECT min(i) FROM t Int64 None

The final diagnostic is {"cases":4,"missing_output_types":4}.

Expected behavior

AggregateFunction.output_type must be set to the return type derived from the referenced function declaration. The producer should write a conforming type, or report that it cannot represent that function contract.

from_aggregate_function currently sets output_type: None explicitly. A consumer that validates required fields can reject these plans; one that uses the declaration for schema inference has no type to use.

This report is about the missing declaration. It does not prescribe an integer AVG return type or address aggregate phases. #24967 and #25045 concern phase handling in the consumer; #15831 fixed the corresponding missing-output-type issue for BinaryExpr.

Activity

  1. namanjain24-sudo commented on Sep 8, 2026

    @namanjain24-sudo
    Contributor

    take

  2. added 4 commits that reference this issue on Sep 11, 2026
    f93a6cb
    40df8f8
    3cc32f3
    3616533
  3. alamb commented on Sep 11, 2026

    @alamb
    Contributor

    Same question: #25190 (comment)

    I don't understand what you are doing, or why this is a bug. Can you please fix clarify on this PR what the actual problem is you are solving?

  4. alexandrefimov commented on Sep 11, 2026

    @alexandrefimov
    ContributorAuthor

    Yes, this is a compatibility problem. I should have included a concrete example.

    DataFusion produces aggregate calls without output_type. Current substrait-java tries to read this field and rejects a minimal sum:i64 plan with:

    UnsupportedOperationException: Type is not set

    The same plan works when output_type is present and produces the expected nullable i64 field. DataFusion-to-DataFusion round trips do not catch this because the problem only shows up when another implementation reads the plan.

    So #25090 fixes plans exported by DataFusion for consumers such as substrait-java.

  5. namanjain24-sudo commented on Sep 11, 2026

    @namanjain24-sudo
    Contributor

    @alamb answered this in full on #25090, since you asked for it there:
    #25090 (comment) — the engine, the two source
    lines it fails on, and a before/after run.

    One thing worth adding to @alexandrefimov's reply, because it matters if you try to reproduce it. The
    error is exactly right for a minimal hand-built sum:i64 plan, but a plan that DataFusion actually
    exports never reaches that check. Handing the output of to_substrait_plan straight to
    substrait-java 0.103.0 fails earlier, at extension resolution:

    IllegalStateException: Function 'sum' references URN anchor -1, but no URN is registered at that anchor
    

    We write extension_urn_reference: u32::MAX for every function declaration (extensions.rs:120),
    which is #11545. Only once a URN is registered on the plan and the compound key sum:i64 is used
    does it get as far as output_type and throw UnsupportedOperationException: Type is not set. With
    this change and that same workaround the plan is accepted and comes back as nullable i64, which is
    what our own optimized plan says sum(t.i) is.

    So it is a real compatibility bug and it is the second of two gates rather than the only one. #11545
    is the first. Fixing either alone still leaves our aggregate plans unreadable by substrait-java.

  6. added 3 commits that reference this issue on Sep 14, 2026
    8d5511a
    f0f8429
    97bdefe
  7. added a commit that references this issue on Sep 23, 2026
    ed2cc50
  8. added 3 commits that reference this issue on Oct 11, 2026
    aa564b3
    bde3629
    d60696c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions