Skip to content

Commit 7cb1d78

Browse files
authored
Resolves #3172: Fix continuation bug in StreamAggregation plan (#3112)
The problem was: continuation is advanced after a group ends, so if we execute the query with the returned continuation, we'll start from the second row in the next group. This PR fixes the problem, and adds more tests.
1 parent d750e02 commit 7cb1d78

File tree

3 files changed

+138
-53
lines changed

3 files changed

+138
-53
lines changed

docs/ReleaseNotes.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ Users performing online updates are encouraged to update from [4.0.559.4](#40559
2222
* **Bug fix** Fix 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
2323
* **Bug fix** Fix 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
2424
* **Bug fix** Fix 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
25-
* **Bug fix** Fix 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
26-
* **Bug fix** Fix 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
25+
* **Bug fix** Fix continuation bug in AggregateCursor when a group is finished [Issue #3172](https://github.com/FoundationDB/fdb-record-layer/issues/3172)
2726
* **Performance** Improvement 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
2827
* **Performance** Improvement 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
2928
* **Performance** Improvement 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/AggregateCursor.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ public CompletableFuture<RecordCursorResult<QueryResult>> onNext() {
8484
} else {
8585
final QueryResult queryResult = Objects.requireNonNull(innerResult.get());
8686
boolean groupBreak = streamGrouping.apply(queryResult);
87-
previousValidResult = innerResult;
87+
if (!groupBreak) {
88+
// previousValidResult is the last row before group break, it sets the continuation
89+
previousValidResult = innerResult;
90+
}
8891
return (!groupBreak);
8992
}
9093
}), getExecutor()).thenApply(vignore -> {
@@ -98,6 +101,29 @@ public CompletableFuture<RecordCursorResult<QueryResult>> onNext() {
98101
}
99102
// Use the last valid result for the continuation as we need non-terminal one here.
100103
RecordCursorContinuation continuation = Verify.verifyNotNull(previousValidResult).getContinuation();
104+
/*
105+
* Update the previousValidResult to the next continuation even though it hasn't been returned. This is to return the correct continuation when there are single-element groups.
106+
* Below is an example that shows how continuation(previousValidResult) moves:
107+
* Initial: previousResult = null, previousValidResult = null
108+
row0 groupKey0 groupBreak = False previousValidResult = row0 previousResult = row0
109+
row1 groupKey0 groupBreak = False previousValidResult = row1 previousResult = row1
110+
row2 groupKey1 groupBreak = True previousValidResult = row1 previousResult = row2
111+
* returns result (groupKey0, continuation = row1), and set previousValidResult = row2
112+
*
113+
* Now there are 2 scenarios, 1) the current iteration continues; 2) the current iteration stops
114+
* In scenario 1, the iteration continues, it gets to row3:
115+
row3 groupKey2 groupBreak = True previousValidResult = row2 previousResult = row3
116+
* returns result (groupKey1, continuation = row2), and set previousValidResult = row3
117+
*
118+
* In scenario 2, a new iteration starts from row2 (because the last returned continuation = row1), and set initial previousResult = null, previousValidResult = null:
119+
row2 groupKey1 groupBreak = False previousValidResult = row2 previousResult = row2
120+
* (Note that because a new iteration starts, groupBreak = False for row2.)
121+
row3 groupKey2 groupBreak = True previousValidResult = row2 previousResult = row3
122+
* returns result (groupKey1, continuation = row2), and set previousValidResult = row3
123+
*
124+
* Both scenarios returns the correct result, and continuation are both set to row3 in the end, row2 is scanned twice if a new iteration starts.
125+
*/
126+
previousValidResult = previousResult;
101127
return RecordCursorResult.withNextValue(QueryResult.ofComputed(streamGrouping.getCompletedGroupResult()), continuation);
102128
});
103129
}

0 commit comments

Comments
 (0)