Skip to content

fix: Derive Substrait intersection nullability from every input - #25091

Open
namanjain24-sudo wants to merge 1 commit into
apache:mainfrom
namanjain24-sudo:fix-substrait-intersection-nullability
Open

fix: Derive Substrait intersection nullability from every input#25091
namanjain24-sudo wants to merge 1 commit into
apache:mainfrom
namanjain24-sudo:fix-substrait-intersection-nullability

Conversation

@namanjain24-sudo

Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

The Substrait consumer derived all three intersection schemas from the primary
input alone, so a field the intersection makes required stayed nullable in the
logical output schema.

The Set Operation rules give intersections a different rule: for the multiset
intersections a field is required as soon as any input requires it, and for
INTERSECTION_PRIMARY it is nullable only when it is nullable in the primary
input and in at least one secondary input.

from_set_rel builds intersections with LogicalPlanBuilder::intersect, which
compiles to a left semi join and therefore keeps the left input's nullability.

What changes are included in this PR?

Intersections now go through a small helper that narrows the result's
nullability to left AND right per field.

The join matches nulls with nulls (NullEquality::NullEqualsNull) on every
field, so a left row holding a null in some field only survives when the right
input holds a null there too — a field is nullable in the result only when both
inputs make it nullable. Applied to each step, that single rule reproduces both
spec rules:

  • the multiset intersections chain pairwise, so the result is required when any
    input requires it;
  • for INTERSECTION_PRIMARY the right side is the union of the secondary
    inputs, whose field is nullable exactly when some secondary makes it nullable,
    which yields "nullable in the primary and in at least one secondary".

When nothing needs narrowing the plan is returned unchanged, so the common
all-nullable case is untouched. Unions and the MINUS operations are not
affected.

What is the testing strategy for this PR?

New test intersect_nullability in datafusion/substrait/tests/cases/logical_plans.rs,
with three plans added under tests/testdata/test_plans/. They intersect three
tables carrying the same four columns with the spec's nullability pattern
(? marks nullable):

primary     a? b? c? d?
secondary   a  b  c? d?
secondary   a  b? c  d?
Operation Result
INTERSECTION_PRIMARY a, b?, c?, d?
INTERSECTION_MULTISET a, b, c, d?
INTERSECTION_MULTISET_ALL a, b, c, d?

The test fails on main (a? where a is expected) and passes here. It also
executes each plan, so the narrowed schema is checked to survive optimization
and execution.

I also ran the probe from the issue. The three setop_intersection_* lines now
match the expected column, and the union and primary-minus controls are
unchanged:

setop_intersection_primary       [c0, c1, c2, c3, c4, c5?, c6?, c7?]
setop_intersection_multiset      [c0, c1, c2, c3, c4, c5, c6, c7?]
setop_intersection_multiset_all  [c0, c1, c2, c3, c4, c5, c6, c7?]

The existing datafusion-substrait suite passes unchanged, including the
intersection roundtrip tests.

Are there any user-facing changes?

Intersections consumed from Substrait now report a narrower, spec-conforming
nullability. No public API changes.

One note for reviewers: the same narrowing would apply to SQL INTERSECT, since
LogicalPlanBuilder::intersect keeps the left nullability for every caller. I
kept this change inside the Substrait consumer to match the scope of the issue
and to avoid changing SQL plans in the same PR. If you would rather see the rule
live in LogicalPlanBuilder, I am happy to move it.

The Substrait consumer derived all three intersection schemas from the
primary input alone, so a field that the intersection makes required
stayed nullable in the logical output schema.

Narrow an intersection's nullability to `left AND right` per field. The
left semi join it compiles to matches nulls with nulls, so a field is
nullable in the result only when both inputs make it nullable, which
reproduces the spec's rule for the multiset intersections and, because
the right side is the union of the secondary inputs, for the primary
intersection as well.

Closes apache#25042.
@github-actions github-actions Bot added the substrait Changes to the substrait crate label Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

substrait Changes to the substrait crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Substrait intersection schemas retain primary-input nullability

1 participant