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 55 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: 0b504df8-36b1-4fb7-95f9-80c5da00e30d

📥 Commits

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

📒 Files selected for processing (11)

• core/src/main/java/io/substrait/relation/AbstractReadRel.java
• core/src/main/java/io/substrait/relation/Join.java
• core/src/main/java/io/substrait/relation/LateralJoin.java
• core/src/main/java/io/substrait/relation/ProtoRelConverter.java
• core/src/main/java/io/substrait/relation/physical/HashJoin.java
• core/src/main/java/io/substrait/relation/physical/MergeJoin.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.

[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.

[bvolpato]

Refreshed against main, keeping read filters before projection and emit. Lateral, hash, and merge post-join filters now use the direct output schema as well. I also updated the compatibility note and the spec reference.

[nielspardon]

Thanks, both are fixed. The rest is small and nothing here blocks.

[bvolpato]

Updated these too. The round-trip test now covers every defined join type, applyFilter is a documented protected hook, and the read/post-join accessors spell out their coordinate systems. FALSE and NULL now run as separate cases, with assertEquals on the empty result and structural correlation checks.