[SQL] GC for JoinFilterMap operator was applied on the wrong side - #6830
Conversation
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
| final DBSPExpression func = new DBSPTupleExpression(monotoneFields, false); | ||
| final OutputPort extractLeft = this.createApply(leftLimiter, join, func.closure(var)); | ||
|
|
||
| if (INSERT_RETAIN_VALUES) { |
There was a problem hiding this comment.
this was the bug: notice how the GC inferred from the left is applied to the left input
there's a similar mistake for the right input
mythical-fred
left a comment
There was a problem hiding this comment.
Correctness fix looks right. The prior code used each side's own delta-path bound to GC that same side's input trace; that only works when the two paths carry the same monotone column, which is not the case for a JoinFilterMap whose inequality relates two different timestamp columns. Trace on side X can only be pruned once we know values on side X can never join a future arrival on side Y, so the retention closure has to be extracted from the delta bound of side Y. Swapping the two if (leftLimiter/rightLimiter) blocks accomplishes exactly that.
The extraction of joinInputRetention(parameterNumber, oppositePathTuple, iomap, valueType, keyPart) replaces the manual varIndex bookkeeping with oppositePathTuple.compressedIndex(outputField). That is cleaner and less error-prone; the two prior copies of that loop had subtly different structures (the right-hand branch pre-walked left fields to advance varIndex), and both are now the same code path.
Tests are excellent: issue6829, issue6829Windowed, issue6829Asof each pair the LATENESS variant against a lateness-free control that must produce identical outputs but must also have zero GC operators. The trace-diagram comments walking through the bound at each step will pay for themselves the next time someone edits this file. checkJoinGC asserts both the presence of the expected retainers and the absence of any others.
APPROVE.
|
I am merging this, since it's a clear bug fix |
Fixes #6829
Checklist