SOLR-18418: Fix complement()/intersect() with asymmetric on= - #4873
Open
dsmiley wants to merge 1 commit into
Open
SOLR-18418: Fix complement()/intersect() with asymmetric on=#4873dsmiley wants to merge 1 commit into
dsmiley wants to merge 1 commit into
Conversation
… asymmetric on= complement() and intersect() streaming expressions silently returned wrong results whenever their on= clause mapped two differently-named fields (e.g. on="_parent_document_id=document_id"), with no exception raised. Root cause: both decorators compared tuples across streamA/streamB using streamA.getStreamSort() - a FieldComparator whose left and right field names are both streamA's own field. Applied to a streamB tuple, this always read a missing field as null, and FieldComparator's null-handling branch returns a constant, non-negative result. The on= mapping was honored by eq.test() but never by the comparison that drives the merge, so a non-matching pair was never recognized as "streamA's value is less", and instead of advancing streamB it discarded it, one tuple at a time. The first streamA value absent from streamB therefore drained streamB to EOF, after which every remaining streamA tuple hit the "streamB is EOF" branch: complement() emitted its entire input and intersect() emitted nothing. Fix: reuse the equalitor-derived comparator that innerJoin/leftOuterJoin/ fullOuterJoin already build correctly (BiJoinStream.createIterationComparator, now promoted to StreamEqualitor.deriveComparator so BiJoinStream, Complement- Stream, and IntersectStream share one implementation). Also: - Added StreamEqualitor.isDerivedFromLeft/isDerivedFromRight so Complement/ IntersectStream's precondition check validates each stream against the correct side of an asymmetric on=, instead of the old isDerivedFrom(), which OR's the two field checks together and can't detect an asymmetric mismatch. - ComplementStream/IntersectStream deduped streamB with the full (asymmetric) equalitor, which compared streamB tuples using streamA's field name and so never matched; fixed via StreamEqualitor.deriveRightEqualitor(eq). - Added StreamEqualitor.assertFieldsPresent(), called at the cross-stream comparison site, to fail loudly if an on= field is entirely absent from a tuple (a wiring bug) rather than silently treating it as null. - Added regression tests using an asymmetric on= where streamA's first value is absent from streamB (the condition that drains streamB), asserting exact membership plus the |complement| + |intersect| == |streamA| invariant, and a focused test for the streamB dedup fix. - Updated the complement/intersect ref guide sections with the sort/on= precondition and the sanity-check invariant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dsmiley
commented
Sep 4, 2026
| streamB.pushBack(b); | ||
| return a; | ||
| if (!eq.test(a, b)) { | ||
| eq.assertFieldsPresent(a, b); |
Contributor
Author
There was a problem hiding this comment.
This check is going to be on every tuple reaching this point (and same for other call-sites in this PR). It has some overhead (hashmap lookups) but it saves frustration / trust issues. I think it's the right trade-off.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
complement() and intersect() streaming expressions silently returned wrong results whenever their on= clause mapped two differently-named fields (e.g. on="_parent_document_id=document_id"), with no exception raised.
Root cause: both decorators compared tuples across streamA/streamB using streamA.getStreamSort() - a FieldComparator whose left and right field names are both streamA's own field. Applied to a streamB tuple, this always read a missing field as null, and FieldComparator's null-handling branch returns a constant, non-negative result. The on= mapping was honored by eq.test() but never by the comparison that drives the merge, so a non-matching pair was never recognized as "streamA's value is less", and instead of advancing streamB it discarded it, one tuple at a time. The first streamA value absent from streamB therefore drained streamB to EOF, after which every remaining streamA tuple hit the "streamB is EOF" branch: complement() emitted its entire input and intersect() emitted nothing.
Fix: reuse the equalitor-derived comparator that innerJoin/leftOuterJoin/ fullOuterJoin already build correctly (BiJoinStream.createIterationComparator, now promoted to StreamEqualitor.deriveComparator so BiJoinStream, Complement- Stream, and IntersectStream share one implementation). Also:
https://issues.apache.org/jira/browse/SOLR-18418
diagnosed and solution/fix written entirely with AI