fix!: preserve mandatory read and post-join filters

[bvolpato]

Mandatory ReadRel filters and JoinRel post-join filters are currently dropped when converting plans to Calcite or Spark. A read carrying FALSE therefore returns its input rows, and an outer join loses predicates that should inspect its null-extended output.

For example, this scan must produce no rows, but Isthmus currently returns an unrestricted TableScan:

NamedScan scan = NamedScan.builder()
    .addNames("t")
    .initialSchema(NamedStruct.of(
        List.of("id"), TypeCreator.REQUIRED.struct(TypeCreator.REQUIRED.I32)))
    .filter(ExpressionCreator.bool(false, false))
    .build();
new SubstraitToCalcite(ConverterProvider.DEFAULT).convert(scan);

Apply mandatory scan filters before projection and emit remapping, and post-join filters after join output formation. Decode logical, lateral, hash, and merge post-join references against each join's direct output schema, preserving outer-join nullability and semi/anti output coordinates. This follows the direct-output rule documented in spec v0.91.0 and the lateral-join semantics in the pinned spec v0.102.0.

Partially addresses #1204 for mandatory read filtering.

BREAKING CHANGE: Mandatory read and post-join filters are now enforced by Calcite and Spark conversion. Post-join field references are decoded against the join's direct output before emit, rather than concatenated inputs. Regenerate plans that reference removed semi/anti input columns or use concatenated-input indices for right-oriented joins; outer-join references now retain their nullable output types.

[alexandrefimov]

Heads-up on an overlap: ReadRel.projection, one of the three fields #1204 names, is in #1280 -- it applies the mask in visit(NamedScan) and visit(VirtualTableScan), where your applyFilter calls also sit.

The two compose: the filter stays on the scan, the mask goes above it, the emit mapping above that -- a filter is read against the direct schema, before the projection (spec v0.102.0, Read Filtering). Whichever of us lands second rebases, and I am glad to be that one.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: substrait-io/substrait-java/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 427031d0-7e0b-47fb-b720-f6f118fa3640

📥 Commits

Reviewing files that changed from the base of the PR and between bc050d3 and 0f9253b.

📒 Files selected for processing (6)

• core/src/main/java/io/substrait/relation/ProtoRelConverter.java
• core/src/test/java/io/substrait/type/proto/JoinRoundtripTest.java
• isthmus/src/main/java/io/substrait/isthmus/SubstraitRelNodeConverter.java
• isthmus/src/test/java/io/substrait/isthmus/EmbeddedPredicateTest.java
• spark/src/main/scala/io/substrait/spark/logical/ToLogicalPlan.scala
• spark/src/test/scala/io/substrait/spark/MandatoryPredicatesSuite.scala

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nielspardon]

Please mark this breaking (fix!: plus a BREAKING CHANGE: footer). Decoding post_join_filter against the join output changes what field indices in existing plans mean: a LEFT_SEMI filter that references a right column now fails to decode, and an outer-join reference changes nullability. The direct-output rule came in spec v0.91.0, so the body can cite that instead of v0.102.0.

[nielspardon]

Apply the same output-schema decoding in newLateralJoin (line 1065), newHashJoin (1141) and newMergeJoin (1180). The spec gives all four joins the same direct-output rule, and right now a LEFT is_null filter on those three decodes with the wrong nullability. JoinRoundtripTest.lateralJoinWithAnchorAndPostFilter builds its filter over left+right, so it will need the same update.

[bvolpato]

Thanks, good catch. I’ve prepared the same output-schema decoding for lateral, hash, and merge joins, while keeping join conditions and residual predicates on the input schema. The regressions cover outer-join nullability, semi/anti output coordinates, and emit mappings; they fail with the previous decoder and pass with the local changes. The full build passes too. These changes are local and have not been pushed yet.

[bvolpato]

Thanks for flagging the projection overlap and the compatibility impact. I prepared a candidate on current main that applies read filters before projection and emit, including named scans and both virtual-table forms. The combined regressions and full build pass locally. I also drafted a breaking-change title and footer, with the direct-output semantics attributed to spec v0.91.0. The code and description updates have not been published yet.