gianm commented on code in PR #14196:
URL: https://github.com/apache/druid/pull/14196#discussion_r1246810324
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/common/SortMergeJoinFrameProcessor.java:
##########
@@ -166,10 +168,10 @@ public List<WritableFrameChannel> outputChannels()
@Override
public ReturnOrAwait<Long> runIncrementally(IntSet readableInputs) throws
IOException
{
- // Fetch enough frames such that each tracker has one readable row.
+ // Fetch enough frames such that each tracker has one readable row (or is
done).
for (int i = 0; i < inputChannels.size(); i++) {
final Tracker tracker = trackers.get(i);
- if (tracker.isAtEndOfPushedData() && !pushNextFrame(i)) {
+ if (tracker.needsMoreDataForCurrentCursor() && !pushNextFrame(i)) {
Review Comment:
It wasn't *really* a bug in the old code, but I think the new code is
clearer, so I made the change for that reason.
The call `tracker.needsMoreDataForCurrentCursor()` is `!done &&
isAtEndOfPushedData()`. So the old code is basically checking this to see if it
needs to wait for more frames:
```
tracker.isAtEndOfPushedData() && !pushNextFrame(i)
```
And the new code is:
```
!tracker.done && tracker.isAtEndOfPushedData() && !pushNextFrame(i)
```
Which makes sense, because if `tracker.done` is true then there is certainly
no reason to get more frames.
But the old code behaves the same way, even without this additional check.
In the case where `tracker.done` is false, the new code clearly behaves the
same as the old code. In the case where `tracker.done` is true, then
`pushNextFrame(i)` would also be true (since `channel.isFinished` must be true
if `tracker.done`) and so the old code still behaves the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
