Skip to content

infer_rel_schema derives SetRel schema from primary input only, ignoring set-operation nullability rules #219

Description

@nielspardon

Summary

type_inference.infer_rel_schema's set branch derives a SetRel's output schema from rel.set.inputs[0] only, and never reads 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 and INTERSECTION variants the inferred nullability can be wrong (too strict).

elif rel_type == "set":
    (common, struct) = (rel.set.common, infer_rel_schema(rel.set.inputs[0]))
    #                                   ^ primary input only; secondaries and op ignored

(Field types are correctly taken from the primary — the spec requires them identical across inputs — so this is a nullability-only defect.)

Spec rules (from Set Operation output nullability)

  • UNION_DISTINCT / UNION_ALL: "If a field is nullable in any of the inputs, it is nullable in the output."
  • INTERSECTION_PRIMARY: "If a field is nullable in the primary input and in any of the secondary inputs, it is nullable in the output."
  • INTERSECTION_MULTISET / INTERSECTION_MULTISET_ALL: "If a field is required in any of the inputs, it is required in the output."
  • MINUS_PRIMARY / MINUS_PRIMARY_ALL / MINUS_MULTISET: "The same as the primary input."

Because MINUS is "same as primary", inputs[0]-only inference happens to be correct for MINUS. The bug bites UNION (should widen required→nullable when any secondary is nullable) and INTERSECTION (has its own combining rule).

Reproduction (verified)

import substrait.algebra_pb2 as stalg
import substrait.type_pb2 as stt
from substrait.type_inference import infer_rel_schema

def read(nullability):
    return stalg.Rel(read=stalg.ReadRel(base_schema=stt.NamedStruct(
        names=["x"],
        struct=stt.Type.Struct(types=[stt.Type(i64=stt.Type.I64(nullability=nullability))]),
    )))

A = read(stt.Type.NULLABILITY_REQUIRED)   # primary: required
B = read(stt.Type.NULLABILITY_NULLABLE)   # secondary: nullable
rel = stalg.Rel(set=stalg.SetRel(inputs=[A, B], op=stalg.SetRel.SET_OP_UNION_ALL))

print(stt.Type.Nullability.Name(infer_rel_schema(rel).types[0].i64.nullability))
# actual:   NULLABILITY_REQUIRED
# expected: NULLABILITY_NULLABLE  (nullable in any input -> nullable out)

Impact / reachability

Unlike #217 (emit on a SetRel, only reachable via foreign plans), this is reachable through the library's own builders: plan.set(...) and the DataFrame union / intersect / except_ verbs build a SetRel with no stored schema, so any later infer_plan_schema on a union/intersection of two sub-plans whose shared columns differ in nullability yields a schema that under-reports nullability. A consumer trusting the inferred schema could treat a column as non-nullable when the set result can contain nulls.

Silent (no error). Severity is moderate: base types are correct, and it only diverges when inputs disagree on nullability for UNION/INTERSECTION.

Suggested direction

In the set branch, infer all rel.set.inputs, then combine per-field nullability according to rel.set.op using the rules above (types taken from the primary; a mismatch in field count/type across inputs could optionally be validated). This needs a small op→rule mapping keyed by SetRel.SetOp name.

Relationship to #217

Independent. #217 / #218 fix the rel.fetch.commonrel.set.common misread in the same branch (dropped emit output_mapping); this issue is the separate nullability-combination gap on the same line. Either can land first.

🤖 Filed with AI assistance.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions