Skip to content

fix: read SetRel.common in set-relation schema inference#218

Merged
tokoko merged 1 commit into
substrait-io:mainfrom
nielspardon:fix/setrel-emit-output-mapping
Jul 16, 2026
Merged

fix: read SetRel.common in set-relation schema inference#218
tokoko merged 1 commit into
substrait-io:mainfrom
nielspardon:fix/setrel-emit-output-mapping

Conversation

@nielspardon

Copy link
Copy Markdown
Member

Problem

type_inference.infer_rel_schema's set branch reads the common from the wrong sub-message:

elif rel_type == "set":
    (common, struct) = (rel.fetch.common, infer_rel_schema(rel.set.inputs[0]))
    #                    ^^^^^^^^^^^^^^^^ SetRel, so this should be rel.set.common

On a set-typed Rel, protobuf returns a default-empty FetchRel for rel.fetch, so common is an empty RelCommon whose emit_kind oneof is unset. The shared emit-application tail then treats it as "direct" and returns the first input's schema verbatim — a SetRel carrying a RelCommon.Emit (output_mapping) has its column remapping silently dropped, yielding a wrong NamedStruct (wrong column order, or wrong count when the mapping selects a subset).

It's a copy-paste from the adjacent fetch branch; every other branch reads rel.<its own type>.common.

Fix

Read the SetRel's own common. One line, plus a regression test (test_inference_set_emit) that builds a SetRel with an output_mapping that reorders and drops a column and asserts the remapped schema — it fails before this change and passes after.

Scope / reachability

Not reachable through this library's own builders: only select() emits an output_mapping, and it places it on a ProjectRel; the set() builder and the union/intersect/except_ DataFrame verbs never set common on a SetRel. So the common build-then-infer path is unaffected. The bug manifests for externally-authored / deserialized / hand-built plans passed to the public inference functions — i.e. interop with plans from other engines.

Testing

pytest — 512 passed, 30 skipped (unchanged); ruff format --check and ruff check clean.

Fixes #217.

🤖 Generated with AI

infer_rel_schema's set branch read rel.fetch.common instead of
rel.set.common, so a SetRel carrying a RelCommon.Emit output_mapping had
its column remapping silently dropped: on a set-typed Rel, rel.fetch is a
default-empty FetchRel whose common has no emit_kind, which the emit tail
treats as "direct" and returns the first input's schema verbatim.

Not reachable through the library's own builders (only select() emits an
output_mapping, and on a ProjectRel); this affects externally-authored or
deserialized plans passed to inference.
@tokoko
tokoko merged commit 0edecbb into substrait-io:main Jul 16, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

infer_rel_schema drops SetRel emit output_mapping (reads rel.fetch.common instead of rel.set.common)

2 participants