Skip to content

Commit 0b736fc

Browse files
zfosterzfoster
authored
[Cosmos] Return undefined for sum aggregate when one partition is undefined (Azure#12988)
* Remove commit revert for sum aggregation PR * sum progress * format * WIP * Adds correct logic for bailing early on undefined group by result * Formats * Adds changelog * Adds tests for other falsy values * formatting
1 parent d6d5163 commit 0b736fc

File tree

4 files changed

+100
-2
lines changed

4 files changed

+100
-2
lines changed

‎sdk/cosmosdb/cosmos/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Release History
22

3+
## 3.9.4 (2021-01-04)
4+
5+
- BUGFIX: Sums group by operations for cross-partition queries correctly with null values
6+
37
## 3.9.3 (2020-10-19)
48

59
- BUGFIX: Fixes bulk operations with top level partitionKey values that are undefined or null.

‎sdk/cosmosdb/cosmos/src/queryExecutionContext/EndpointComponent/GroupByValueEndpointComponent.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ export class GroupByValueEndpointComponent implements ExecutionContext {
6666

6767
if (this.aggregateType) {
6868
const aggregateResult = extractAggregateResult(payload[0]);
69+
// if aggregate result is null, we need to short circuit aggregation and return undefined
70+
if (aggregateResult === null) {
71+
this.completed = true;
72+
}
6973
this.aggregators.get(grouping).aggregate(aggregateResult);
7074
} else {
7175
// Queries with no aggregates pass the payload directly to the aggregator
@@ -75,7 +79,11 @@ export class GroupByValueEndpointComponent implements ExecutionContext {
7579
}
7680
}
7781

78-
// It no results are left in the underling execution context, convert our aggregate results to an array
82+
// We bail early since we got an undefined result back `[{}]`
83+
if (this.completed) {
84+
return { result: undefined, headers: aggregateHeaders };
85+
}
86+
// If no results are left in the underlying execution context, convert our aggregate results to an array
7987
for (const aggregator of this.aggregators.values()) {
8088
this.aggregateResultArray.push(aggregator.getResult());
8189
}

‎sdk/cosmosdb/cosmos/src/queryExecutionContext/EndpointComponent/emptyGroup.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ export const emptyGroup = "__empty__";
88
// Newer API versions rewrite the query to return `item2`. It fixes some legacy issues with the original `item` result
99
// Aggregator code should use item2 when available
1010
export const extractAggregateResult = (payload: any) =>
11-
payload.item2 ? payload.item2 : payload.item;
11+
Object.keys(payload).length > 0 ? (payload.item2 ? payload.item2 : payload.item) : null;

‎sdk/cosmosdb/cosmos/test/public/functional/query.spec.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,5 +124,91 @@ describe("Queries", function() {
124124
"second batch element should be doc3"
125125
);
126126
});
127+
128+
describe("SUM query iterator", function() {
129+
this.timeout(process.env.MOCHA_TIMEOUT || 30000);
130+
131+
it("returns undefined sum with null value in aggregator", async function() {
132+
const container = await getTestContainer(
133+
"Validate QueryIterator Functionality",
134+
undefined,
135+
{
136+
throughput: 10100,
137+
partitionKey: "/id"
138+
}
139+
);
140+
await container.items.create({ id: "5eded6f8asdfasdfasdfaa21be0109ae34e29", age: 22 });
141+
await container.items.create({ id: "5eded6f8a21be0109ae34e29", age: 22 });
142+
await container.items.create({ id: "5edasdfasdfed6f8a21be0109ae34e29", age: null });
143+
await container.items.create({ id: "5eded6f8a2dd1be0109ae34e29", age: 22 });
144+
await container.items.create({ id: "AndersenFamily" });
145+
await container.items.create({ id: "1" });
146+
147+
const queryIterator = container.items.query("SELECT VALUE SUM(c.age) FROM c");
148+
const { resources: sum } = await queryIterator.fetchAll();
149+
assert.equal(sum.length, 0);
150+
});
151+
it("returns undefined sum with false value in aggregator", async function() {
152+
const container = await getTestContainer(
153+
"Validate QueryIterator Functionality",
154+
undefined,
155+
{
156+
throughput: 10100,
157+
partitionKey: "/id"
158+
}
159+
);
160+
await container.items.create({ id: "5eded6f8asdfasdfasdfaa21be0109ae34e29", age: 22 });
161+
await container.items.create({ id: "5eded6f8a21be0109ae34e29", age: 22 });
162+
await container.items.create({ id: "5edasdfasdfed6f8a21be0109ae34e29", age: false });
163+
await container.items.create({ id: "5eded6f8a2dd1be0109ae34e29", age: 22 });
164+
await container.items.create({ id: "AndersenFamily" });
165+
await container.items.create({ id: "1" });
166+
167+
const queryIterator = container.items.query("SELECT VALUE SUM(c.age) FROM c");
168+
const { resources: sum } = await queryIterator.fetchAll();
169+
assert.equal(sum.length, 0);
170+
});
171+
it("returns undefined sum with empty array value in aggregator", async function() {
172+
const container = await getTestContainer(
173+
"Validate QueryIterator Functionality",
174+
undefined,
175+
{
176+
throughput: 10100,
177+
partitionKey: "/id"
178+
}
179+
);
180+
await container.items.create({ id: "5eded6f8asdfasdfasdfaa21be0109ae34e29", age: 22 });
181+
await container.items.create({ id: "5eded6f8a21be0109ae34e29", age: 22 });
182+
await container.items.create({ id: "5edasdfasdfed6f8a21be0109ae34e29", age: [] });
183+
await container.items.create({ id: "5eded6f8a2dd1be0109ae34e29", age: 22 });
184+
await container.items.create({ id: "AndersenFamily" });
185+
await container.items.create({ id: "1" });
186+
187+
const queryIterator = container.items.query("SELECT VALUE SUM(c.age) FROM c");
188+
const { resources: sum } = await queryIterator.fetchAll();
189+
assert.equal(sum.length, 0);
190+
});
191+
it("returns a valid sum with undefined value in aggregator", async function() {
192+
const container = await getTestContainer(
193+
"Validate QueryIterator Functionality",
194+
undefined,
195+
{
196+
throughput: 10100,
197+
partitionKey: "/id"
198+
}
199+
);
200+
await container.items.create({ id: "5eded6f8asdfasdfasdfaa21be0109ae34e29", age: 22 });
201+
await container.items.create({ id: "5eded6f8a21be0109ae34e29", age: 22 });
202+
await container.items.create({ id: "5edasdfasdfed6f8a21be0109ae34e29", age: undefined });
203+
await container.items.create({ id: "5eded6f8a2dd1be0109ae34e29", age: 22 });
204+
await container.items.create({ id: "AndersenFamily" });
205+
await container.items.create({ id: "1" });
206+
207+
const queryIterator = container.items.query("SELECT VALUE SUM(c.age) FROM c");
208+
const { resources: sum } = await queryIterator.fetchAll();
209+
assert.equal(sum.length, 1);
210+
assert.equal(sum[0], 66);
211+
});
212+
});
127213
});
128214
});

0 commit comments

Comments
 (0)