feat: clarify post_join_filter. add residual_expressions to hash join and merge join by yongchul · Pull Request #1044 · substrait-io/substrait

yongchul · 2026-04-13T05:39:39Z

Background

JoinRel, HashJoinRel, and MergeJoinRel have a field namedpost_join_filter, causing confusion. The confusing point is whether post_join_filter is part of the join predicate (i.e., two rows are matched when it evaluates to true) or not (i.e., as the name says, this is post join filter, thus filter evaluates after the join operation).

Substrait FAQ calls out that the post_join_filter is part of join predicate but @yongchul was deeply discontent about the explanation and the naming.

After lengthy discussion in the Slack channel, #807 was created. After more back-and-forth, the original intent of post_join_filter was indeed post join filter, not part of join predicate.

Also, it appears that Calcite has precisely the same name and same semantic, post join filter. Following code point shows placing the filter on top of the join.
https://github.com/apache/calcite/blob/0a39568b167592ded8db1128b5838982ffe264f3/core/src/main/java/org/apache/calcite/rel/rules/LoptOptimizeJoinRule.java#L553-L559

What this changes do?

TL;DR; Make post_join_filter as post_join_filter again.

Documents post_join_filter is not join condition but semantically filter on top of join.
Introduce residual_expression to HashJoinRel and MergeJoinRel so that they can express non-equality join predicate -- I guess this was the confusion who tried to shove non-equality join predicate to HashJoin and MergeJoin somewhere and distorted the meaning without much thought.
Drop equi from hash join and merge join documentation as with residual_expression they are capable of supporting arbitrary join predicate.
Fix the FAQ.

Breaking change?

This is debatable. According to previous FAQ, post_join_filter was residual_expression introduced in this PR. If we take the FAQ as part of the "spec", then this is a breaking change.

If we don't consider FAQ as a spec, then this is not a breaking change but clarification and adding a new feature -- HashJoinRel, MergeJoinRel now supports arbitrary join predicate.

AI disclaimer

The PR is assisted by Claud Opus 4.6.

This change is

westonpace · 2026-04-15T19:04:56Z

+  // An optional boolean filter applied to each output record after join
+  // matching and null-padding (for outer/mark joins) are complete.
+  // Semantically equivalent to placing a FilterRel directly above this join.
  Expression post_join_filter = 6;


Is there any known engine that has this feature?

Great question. I know one that I can fix (it is under my control lol). I will check data fusion, duckdb, and velox.

@benbellick @vbarua @nielspardon any impact on your sides?

datafusion: not supported

duckdb: does not populate post_join_filter

gluten: does use post_join_filter but not sure whether it is consistent with this PR.

The engines we are currently dealing with would also handle this as a separate FilterRel instead of merging this into the Join operation. We may have some use cases in the future for representing fused operators in Substrait though I would look at this separately.

For me I'm fine with both this improved documentation or removing it.

I just checked and we do not use it internally.

Clarifying the documentation is a good idea, though I would prefer removing it if no user of substrait is actually using it.

@nielspardon @benbellick any final thoughts on the PR? This one I can wait until next week meeting.

jacques-n

lgtm

benbellick

Generally looks good to me, just a few small comments!

benbellick · 2026-05-05T16:07:04Z


-  // Specifies which side of input to build the hash table for this hash join. Default is `BUILD_INPUT_RIGHT`.
+  // Specifies which side of input to build the hash table for this hash join.
+  // Defaults to `BUILD_INPUT_RIGHT`.


Does this mean that BUILD_INPUT_UNSPECIFIED => BUILD_INPUT_RIGHT?

I assume that reason for the presence of UNSPECIFIED is to follow the protobuf best practices, but the protobuf docs are pretty explicit that the unspecified value should not have any meaningful semantics:

It may be tempting to declare this default as a semantically meaningful value but as a general rule, do not, to aid in the evolution of your protocol as new enum values are added over time.

This doesn't have to be for this PR, but I think we should should force producers to specify what they mean explicitly. Maybe we do that in one larger PR next.

I made an issue to track this: #1069

Does this mean that BUILD_INPUT_UNSPECIFIED => BUILD_INPUT_RIGHT?

Yes, for the backward compatibility. Mandating would be a breaking change and we can do it in another PR as we discussed in the last meeting.

benbellick · 2026-05-05T16:42:14Z

@@ -23,7 +23,8 @@ The hash equijoin join operator will build a hash table out of one input (defaul
 | Build Input         | Specifies which input is the `Build`.                                                                                                                                                                                   | Optional, defaults to build `Right`, probe `Left`. |


FYI these tables still only discuss left / right keys (deprecated) rather than comparison join keys.

We can address in another PR if you'd like. Though I would love to just get rid of the deprecated keys instead.

Let me see whether I make the change easily in this PR. otherwise, I'll follow up with another one.

Looked into it and looks like better be handled in a separate PR.

Created #1070

…and mergejoin

CLAassistant · 2026-05-05T22:47:17Z

All committers have signed the CLA.

…ce resolution.

benbellick

LGTM!

yongchul requested review from EpsilonPrime, benbellick, cpcloud, jacques-n, nielspardon, vbarua and westonpace as code owners April 13, 2026 05:39

yongchul mentioned this pull request Apr 13, 2026

feat: clarify the expected behavior, and rationale, of the post join filter #807

Closed

nielspardon reviewed Apr 13, 2026

View reviewed changes

Comment thread site/docs/faq.md

nielspardon reviewed Apr 13, 2026

View reviewed changes

Comment thread site/docs/relations/physical_relations.md Outdated

yongchul added the PMC Ready PRs ready for review by PMCs label Apr 13, 2026

westonpace reviewed Apr 15, 2026

View reviewed changes

jacques-n approved these changes Apr 21, 2026

View reviewed changes

benbellick reviewed May 5, 2026

View reviewed changes

yongchul added 3 commits May 5, 2026 09:48

feat: clarify post_join_filter. add residual_expressions to hashjoin …

d3d6882

…and mergejoin

Rewords for clarity.

93d3c8c

drop equi from hashjoin and mergejoin

c0a4dc1

benbellick mentioned this pull request May 5, 2026

Clarify and migrate semantically meaningful *_UNSPECIFIED enum defaults #1069

Open

yongchul force-pushed the residual_join_expression branch from 972ccfa to 1cd5275 Compare May 5, 2026 22:47

yongchul force-pushed the residual_join_expression branch from 1cd5275 to 5ae9c17 Compare May 5, 2026 22:57

Address comments. Clarify post_join_filter semantic and field referen…

1a3c564

…ce resolution.

yongchul force-pushed the residual_join_expression branch 2 times, most recently from cff0ed1 to 1a3c564 Compare May 5, 2026 23:31

benbellick approved these changes May 6, 2026

View reviewed changes

		@@ -23,7 +23,8 @@ The hash equijoin join operator will build a hash table out of one input (defaul
		\| Build Input \| Specifies which input is the `Build`. \| Optional, defaults to build `Right`, probe `Left`. \|

Conversation

yongchul commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

What this changes do?

Breaking change?

AI disclaimer

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongchul Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nielspardon Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacques-n left a comment

Choose a reason for hiding this comment

Uh oh!

benbellick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CLAassistant commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benbellick left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

yongchul commented Apr 13, 2026 •

edited

Loading

yongchul Apr 15, 2026 •

edited

Loading

nielspardon Apr 17, 2026 •

edited

Loading

CLAassistant commented May 5, 2026 •

edited

Loading