Skip to content

Commit 12693c0

Browse files
committed
Fixes FoundationDB#3096: Infinite continuations: sum(*) returns alternating rows forever
1 parent 3b3f341 commit 12693c0

File tree

2 files changed

+71
-15
lines changed

2 files changed

+71
-15
lines changed

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryStreamingAggregationPlan.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ public PRecordQueryStreamingAggregationPlan toProto(@Nonnull final PlanSerializa
370370
}
371371
builder.setGroupingKeyAlias(groupingKeyAlias.getId())
372372
.setAggregateAlias(aggregateAlias.getId())
373-
.setCompleteResultValue(completeResultValue.toValueProto(serializationContext));
373+
.setCompleteResultValue(completeResultValue.toValueProto(serializationContext))
374+
.setIsCreateDefaultOnEmpty(isCreateDefaultOnEmpty);
374375
return builder.build();
375376
}
376377

yaml-tests/src/test/resources/aggregate-index-tests.yamsql

+69-14
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,22 @@ test_block:
132132
# At some point, should be able to roll up values from the aggregate index. However, even
133133
# controlling for that, it can still use the index
134134
- query: select max(col2) from T1 use index (mv8);
135+
- supported_version: !current_version
135136
- explain: "ISCAN(MV8 <,>) | MAP (_ AS _0) | AGG (max_l(_._0.COL2) AS _0) | ON EMPTY NULL | MAP (_._0._0 AS _0)"
136-
# 4.1 Triage: failed when running with forced continuations (Received continuation shouldn't be at beginning)
137-
# - result: [{!l 13}]
137+
- result: [{!l 13}]
138+
-
139+
- query: select max(col2) from T1 use index (mv8);
140+
- maxRows: 1
141+
# Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
142+
# (Extra values being produced after exhausting source of an aggregate cursor)
143+
# Can remove once we do not care about backwards compatibility before !current_version
144+
- initialVersionLessThan: !current_version
145+
- result: [{!l 13}]
146+
- result: [{!null _}]
147+
- result: [{!l 13}] # Repeats ad infinitum
148+
- initialVersionAtLeast: !current_version
149+
- result: [{!l 13}]
150+
- result: []
138151
-
139152
# Min/max indexes need keep what amounts to a standard value index on their keys (in order to properly look up
140153
# the min/max). That index should be usable for normal queries just like a value index. Note that the scan is
@@ -148,16 +161,42 @@ test_block:
148161
- result: [{!l 5}, {!l 4}, {!l 3}, {!l 2}, {!l 1}]
149162
-
150163
- query: select min(col3) from T2 group by col1, col2;
164+
- supported_version: !current_version
151165
- explain: "ISCAN(MV2 <,>) | MAP (_ AS _0) | AGG (min_l(_._0.COL3) AS _0) GROUP BY (_._0.COL1 AS _0, _._0.COL2 AS _1) | MAP (_._1._0 AS _0)"
152-
# 4.1 Triage: failed when running with forced continuations (result mismatch)
153-
# - result: [{!l 1}, {!l 2}, {!l 3}]
166+
- result: [{!l 1}, {!l 2}, {!l 3}]
167+
-
168+
- query: select min(col3) from T2 group by col1, col2;
169+
# Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
170+
# (Extra values being produced after exhausting source of an aggregate cursor)
171+
# Can remove once we do not care about backwards compatibility before !current_version
172+
- maxRows: 1
173+
- initialVersionLessThan: !current_version
174+
- initialVersionAtLeast: !current_version
175+
- result: [{!l 1}]
176+
- result: [{!l 2}]
177+
- result: [{!l 3}]
178+
- result: []
154179
-
155180
# this should use the aggregate index in the future, for now, it is using streaming aggregate
156181
# over base table scan.
157182
- query: select max(col2) from t2;
183+
- supported_version: !current_version
158184
- explain: "ISCAN(MV3 <,>) | MAP (_ AS _0) | AGG (max_l(_._0.COL2) AS _0) | ON EMPTY NULL | MAP (_._0._0 AS _0)"
159-
# 4.1 Triage: failed when running with forced continuations (result mismatch)
160-
# - result: [{!l 2}]
185+
- result: [{!l 2}]
186+
-
187+
- query: select max(col2) from t2;
188+
# Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
189+
# (Extra values being produced after exhausting source of an aggregate cursor)
190+
# Can remove once we do not care about backwards compatibility before !current_version
191+
- supported_version: !current_version
192+
- maxRows: 1
193+
- initialVersionLessThan: !current_version
194+
- result: [{!l 2}]
195+
- result: [{!null _}]
196+
- result: [{!l 2}] # ad infinitum
197+
- initialVersionAtLeast: !current_version
198+
- result: [{!l 2}]
199+
- result: []
161200
-
162201
- query: select col1, sum(col2) from T1 USE INDEX (vi1) group by col1;
163202
- explain: "ISCAN(VI1 <,>) | MAP (_ AS _0) | AGG (sum_l(_._0.COL2) AS _0) GROUP BY (_._0.COL1 AS _0) | MAP (_._0._0 AS COL1, _._1._0 AS _1)"
@@ -211,14 +250,28 @@ test_block:
211250
-
212251
# Permuted max index can also be used to evaluate other aggregate functions via aggregation and roll-up
213252
- query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 asc;
253+
- supported_version: !current_version
214254
- explain: "ISCAN(MV9 [EQUALS promote(@c20 AS LONG)]) | MAP (_ AS _0) | AGG (sum_l(_._0.COL2) AS _0) GROUP BY (_._0.COL1 AS _0, _._0.COL3 AS _1) | MAP (_._0._1 AS COL3, _._1._0 AS S)"
215-
# 4.1 Triage: failed when running with forced continuations (result mismatch)
216-
# - result: [{COL3: 1, S: 1}, {COL3: 2, S: 2}, {COL3: 100, S: 1}, {COL3: 200, S: 2}]
255+
- result: [{COL3: 1, S: 1}, {COL3: 2, S: 2}, {COL3: 100, S: 1}, {COL3: 200, S: 2}]
256+
-
257+
- query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 asc;
258+
# Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
259+
# (Extra values being produced after exhausting source of an aggregate cursor)
260+
# Can remove once we do not care about backwards compatibility before !current_version
261+
- maxRows: 0
262+
- result: [{COL3: 1, S: 1}, {COL3: 2, S: 2}, {COL3: 100, S: 1}, {COL3: 200, S: 2}]
217263
-
218264
- query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 desc;
265+
- supported_version: !current_version
219266
- explain: "ISCAN(MV9 [EQUALS promote(@c20 AS LONG)] REVERSE) | MAP (_ AS _0) | AGG (sum_l(_._0.COL2) AS _0) GROUP BY (_._0.COL1 AS _0, _._0.COL3 AS _1) | MAP (_._0._1 AS COL3, _._1._0 AS S)"
220-
# 4.1 Triage: failed when running with forced continuations (result mismatch)
221-
# - result: [{COL3: 200, S: 2}, {COL3: 100, S: 1}, {COL3: 2, S: 2}, {COL3: 1, S: 1}]
267+
- result: [{COL3: 200, S: 2}, {COL3: 100, S: 1}, {COL3: 2, S: 2}, {COL3: 1, S: 1}]
268+
-
269+
- query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 desc;
270+
# Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
271+
# (Extra values being produced after exhausting source of an aggregate cursor)
272+
# Can remove once we do not care about backwards compatibility before !current_version
273+
- maxRows: 0
274+
- result: [{COL3: 200, S: 2}, {COL3: 100, S: 1}, {COL3: 2, S: 2}, {COL3: 1, S: 1}]
222275
# -
223276
# # grouping by constant is not yet supported.
224277
# - query: select sum(col2) from t1 group by 3,2,1;
@@ -233,10 +286,12 @@ test_block:
233286
-
234287
- query: select max_ever(col3) from T2 group by col1, col2;
235288
- result: [{!l 100}, {!l 200}, {!l 400}]
236-
# 4.1 Triage: failed when running with forced continuations (Received continuation shouldn't be at beginning)
237-
# -
238-
# - query: select min_ever(col3) from t2
239-
# - result: [{!l 1}]
289+
-
290+
- query: select min_ever(col3) from t2
291+
# Cannot enable FORCE_CONTINUATIONS with ungrouped aggregate scan because of: https://github.com/FoundationDB/fdb-record-layer/issues/3206
292+
- maxRows: 0
293+
- explain: "AISCAN(MV7 <,> BY_GROUP -> [_0: VALUE:[0]]) | MAP (_ AS _0) | ON EMPTY NULL | MAP (_._0._0 AS _0)"
294+
- result: [{!l 1}]
240295
-
241296
- query: select min_ever(col3) from t2
242297
- explain: "AISCAN(MV7 <,> BY_GROUP -> [_0: VALUE:[0]]) | MAP (_ AS _0) | ON EMPTY NULL | MAP (_._0._0 AS _0)"

0 commit comments

Comments
 (0)