Skip to content

infer_expression_type takes IfThen and SwitchExpression nullability from the first branch only #277

Description

@nielspardon

infer_expression_type derives an IfThen's type from ifs[0].then alone (src/substrait/type_inference.py:426) and a SwitchExpression's from switch_expression.ifs[0].then (type_inference.py:433). The remaining then branches and the else branch are never consulted, so when the branches disagree on nullability the answer depends on branch order — and in one order it is unsound, declaring REQUIRED for an expression that can evaluate to null.

A COALESCE-style rewrite over a left join is exactly this shape: one branch is a required column, the other a null-padded one.

Reproducer

import substrait.algebra_pb2 as stalg, substrait.type_pb2 as stt
from substrait.type_inference import infer_expression_type

R, N = stt.Type.NULLABILITY_REQUIRED, stt.Type.NULLABILITY_NULLABLE
NAMES = {R: "REQUIRED", N: "NULLABLE"}

# An input row where field 0 is required and field 1 is nullable.
scope = stt.Type.Struct(
    types=[stt.Type(i64=stt.Type.I64(nullability=R)), stt.Type(i64=stt.Type.I64(nullability=N))],
    nullability=R,
)


def ref(i):
    return stalg.Expression(selection=stalg.Expression.FieldReference(
        root_reference=stalg.Expression.FieldReference.RootReference(),
        direct_reference=stalg.Expression.ReferenceSegment(
            struct_field=stalg.Expression.ReferenceSegment.StructField(field=i))))


def if_then(then, else_):
    true = stalg.Expression(literal=stalg.Expression.Literal(boolean=True))
    return stalg.Expression(if_then=stalg.Expression.IfThen(
        ifs=[stalg.Expression.IfThen.IfClause(**{"if": true, "then": then})],
        **{"else": else_}))


for label, expr in (
    ("then=f0 REQUIRED, else=f1 NULLABLE", if_then(ref(0), ref(1))),
    ("then=f1 NULLABLE, else=f0 REQUIRED", if_then(ref(1), ref(0))),
):
    t = infer_expression_type(expr, scope)
    print(f"{label}: {NAMES[getattr(t, t.WhichOneof('kind')).nullability]}")
then=f0 REQUIRED, else=f1 NULLABLE: REQUIRED
then=f1 NULLABLE, else=f0 REQUIRED: NULLABLE

Both spellings can return null, so the first is wrong. A SwitchExpression with the same branches behaves identically.

Measured on main at afa3c52. Pre-existing.

The output nullability should be nullable if any branch — including else — is nullable. This is the same shape as the Expand switching-field gap in #269 and the grouping-key gap in #268.

🤖 Filed with AI assistance as a follow-up to review of #272.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    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