Skip to content

fix: combine set-operation nullability across all inputs#221

Merged
tokoko merged 1 commit into
substrait-io:mainfrom
nielspardon:fix/setrel-nullability-combination
Jul 16, 2026
Merged

fix: combine set-operation nullability across all inputs#221
tokoko merged 1 commit into
substrait-io:mainfrom
nielspardon:fix/setrel-nullability-combination

Conversation

@nielspardon

@nielspardon nielspardon commented Jul 16, 2026

Copy link
Copy Markdown
Member

Problem

type_inference.infer_rel_schema's set branch derived a SetRel's output schema from rel.set.inputs[0] only and never read rel.set.op. Per the Substrait spec, set inputs share field types but may differ in nullability, and the output nullability must be combined across all inputs according to the operation. So for UNION / INTERSECTION, the inferred nullability could be wrong (too strict) when inputs disagree.

Reachable through this library's own builders: plan.set(...) and the DataFrame union / intersect / except_ verbs build a SetRel with no stored schema, so a later infer_plan_schema over a union of two sub-plans whose shared columns differ in nullability under-reported nullability.

Fix

Infer every input and combine each field's nullability per the operation, matching the spec's set-operation output-type derivation:

  • UNION_DISTINCT / UNION_ALL — nullable if the field is nullable in any input.
  • INTERSECTION_PRIMARY — nullable only when the field is nullable in the primary and in at least one secondary; required otherwise.
  • INTERSECTION_MULTISET / _ALL — required if the field is required in any input.
  • MINUS_PRIMARY / _PRIMARY_ALL / _MULTISET (and unspecified) — same as the primary.

Field types are taken from the primary input (the spec requires identical field types across inputs); only the top-level nullability is recombined, and CopyFrom preserves type parameters and nested element types.

Testing

  • test_inference_set_nullability — parametrized over all 8 concrete set ops using the spec's own worked example (three inputs, one column per required/nullable combination) and asserting the spec's exact output columns.
  • test_inference_set_nullability_preserves_field_types — a UNION_ALL over decimal(10,2) / varchar(5) / list<string> columns, asserting the full Type.Struct so type parameters, nested element types, and struct-level nullability are all verified.
  • Full suite: 521 passed, 30 skipped; ruff format --check and ruff check clean.

An adversarial cross-check (independent re-derivation of all 9 SetOp cases against the spec, plus edge-case and test-completeness passes) confirmed the rules; a defensive guard was added so a field Type with no kind set passes through unchanged rather than raising.

Relationship to #206 / #220

Independent. #220 (registry scoping) also touches the set branch line to thread registry=registry; whichever merges second is a trivial rebase (keep both changes).

Closes #219.

🤖 Generated with AI

infer_rel_schema derived a SetRel's output schema from rel.set.inputs[0]
only and never read rel.set.op, so for UNION/INTERSECTION the output
nullability could be wrong (too strict) when inputs disagree on a field's
nullability. This is reachable via the union/intersect/except_ verbs,
whose SetRel stores no schema.

Infer every input and combine each field's nullability per the operation
(matching the Substrait spec's set-operation output-type derivation):
UNION -> nullable if nullable in any input; INTERSECTION_PRIMARY ->
nullable only when nullable in the primary and in some secondary
(required otherwise); INTERSECTION_MULTISET(_ALL) -> required if required
in any input; MINUS_* -> same as the primary. Field types are taken from
the primary (the spec requires identical field types across inputs).

Closes substrait-io#219.
@nielspardon
nielspardon force-pushed the fix/setrel-nullability-combination branch from ad030d9 to a6f9141 Compare July 16, 2026 13:53
@tokoko
tokoko merged commit be531c4 into substrait-io:main Jul 16, 2026
21 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 derives SetRel schema from primary input only, ignoring set-operation nullability rules

2 participants