diff --git a/.changeset/fix-min-max-multiplicity.md b/.changeset/fix-min-max-multiplicity.md new file mode 100644 index 0000000..ee24463 --- /dev/null +++ b/.changeset/fix-min-max-multiplicity.md @@ -0,0 +1,6 @@ +--- +'@electric-sql/d2ts': patch +'@electric-sql/d2mini': patch +--- + +fix min and max groupBy aggregates to ignore retracted values (negative multiplicity) diff --git a/packages/d2mini/src/operators/groupBy.ts b/packages/d2mini/src/operators/groupBy.ts index 172b66e..cf229b2 100644 --- a/packages/d2mini/src/operators/groupBy.ts +++ b/packages/d2mini/src/operators/groupBy.ts @@ -217,9 +217,14 @@ export function min( return { preMap: (data: T) => valueExtractor(data), reduce: (values: [number, number][]) => { + const net = new Map() + for (const [value, multiplicity] of values) { + net.set(value, (net.get(value) || 0) + multiplicity) + } + let minValue = Number.POSITIVE_INFINITY - for (const [value, _multiplicity] of values) { - if (value < minValue) { + for (const [value, multiplicity] of net) { + if (multiplicity > 0 && value < minValue) { minValue = value } } @@ -238,9 +243,14 @@ export function max( return { preMap: (data: T) => valueExtractor(data), reduce: (values: [number, number][]) => { + const net = new Map() + for (const [value, multiplicity] of values) { + net.set(value, (net.get(value) || 0) + multiplicity) + } + let maxValue = Number.NEGATIVE_INFINITY - for (const [value, _multiplicity] of values) { - if (value > maxValue) { + for (const [value, multiplicity] of net) { + if (multiplicity > 0 && value > maxValue) { maxValue = value } } diff --git a/packages/d2mini/tests/operators/groupBy.test.ts b/packages/d2mini/tests/operators/groupBy.test.ts index 5911597..d22fcff 100644 --- a/packages/d2mini/tests/operators/groupBy.test.ts +++ b/packages/d2mini/tests/operators/groupBy.test.ts @@ -511,6 +511,89 @@ describe('Operators', () => { ] expect(latestMessage.getInner()).toEqual(expectedResult) + + // --- Add a new record --- + input.sendData( + new MultiSet([ + [{ category: 'A', amount: 25 }, 1], + [{ category: 'C', amount: 50 }, 1], + ]), + ) + graph.run() + + const expectedAddResult = [ + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 20, + }, + ], + -1, + ], + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 25, + }, + ], + 1, + ], + [ + [ + '{"category":"C"}', + { + category: 'C', + minimum: 50, + maximum: 50, + }, + ], + 1, + ], + ] + + expect(latestMessage.getInner()).toEqual(expectedAddResult) + + // --- Delete the current min and max --- + input.sendData( + new MultiSet([ + [{ category: 'A', amount: 5 }, -1], + [{ category: 'A', amount: 25 }, -1], + ]), + ) + graph.run() + + const expectedDeleteResult = [ + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 25, + }, + ], + -1, + ], + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 10, + maximum: 20, + }, + ], + 1, + ], + ] + + expect(latestMessage.getInner()).toEqual(expectedDeleteResult) }) test('with median and mode aggregates', () => { diff --git a/packages/d2ts/src/operators/groupBy.ts b/packages/d2ts/src/operators/groupBy.ts index b36bbbe..c839198 100644 --- a/packages/d2ts/src/operators/groupBy.ts +++ b/packages/d2ts/src/operators/groupBy.ts @@ -206,9 +206,14 @@ export function min( return { preMap: (data: T) => valueExtractor(data), reduce: (values: [number, number][]) => { + const net = new Map() + for (const [value, multiplicity] of values) { + net.set(value, (net.get(value) || 0) + multiplicity) + } + let minValue = Number.POSITIVE_INFINITY - for (const [value, _multiplicity] of values) { - if (value < minValue) { + for (const [value, multiplicity] of net) { + if (multiplicity > 0 && value < minValue) { minValue = value } } @@ -227,9 +232,14 @@ export function max( return { preMap: (data: T) => valueExtractor(data), reduce: (values: [number, number][]) => { + const net = new Map() + for (const [value, multiplicity] of values) { + net.set(value, (net.get(value) || 0) + multiplicity) + } + let maxValue = Number.NEGATIVE_INFINITY - for (const [value, _multiplicity] of values) { - if (value > maxValue) { + for (const [value, multiplicity] of net) { + if (multiplicity > 0 && value > maxValue) { maxValue = value } } diff --git a/packages/d2ts/tests/operators-sqlite/groupBy.test.ts b/packages/d2ts/tests/operators-sqlite/groupBy.test.ts index 9ab448b..af02fbf 100644 --- a/packages/d2ts/tests/operators-sqlite/groupBy.test.ts +++ b/packages/d2ts/tests/operators-sqlite/groupBy.test.ts @@ -512,6 +512,93 @@ describe('SQLite Operators', () => { ] expect(latestMessage.collection.getInner()).toEqual(expectedResult) + + // --- Add a new record --- + input.sendData( + v([3, 0]), + new MultiSet([ + [{ category: 'A', amount: 25 }, 1], + [{ category: 'C', amount: 50 }, 1], + ]), + ) + input.sendFrontier(new Antichain([v([4, 0])])) + graph.run() + + const expectedAddResult = [ + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 25, + }, + ], + 1, + ], + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 20, + }, + ], + -1, + ], + [ + [ + '{"category":"C"}', + { + category: 'C', + minimum: 50, + maximum: 50, + }, + ], + 1, + ], + ] + + expect(latestMessage.collection.getInner()).toEqual(expectedAddResult) + + // --- Delete the current min and max --- + input.sendData( + v([5, 0]), + new MultiSet([ + [{ category: 'A', amount: 5 }, -1], + [{ category: 'A', amount: 25 }, -1], + ]), + ) + input.sendFrontier(new Antichain([v([6, 0])])) + graph.run() + + const expectedDeleteResult = [ + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 10, + maximum: 20, + }, + ], + 1, + ], + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 25, + }, + ], + -1, + ], + ] + + expect(latestMessage.collection.getInner()).toEqual(expectedDeleteResult) }) test('with median and mode aggregates', () => { diff --git a/packages/d2ts/tests/operators/groupBy.test.ts b/packages/d2ts/tests/operators/groupBy.test.ts index 13a86e2..d5e44bf 100644 --- a/packages/d2ts/tests/operators/groupBy.test.ts +++ b/packages/d2ts/tests/operators/groupBy.test.ts @@ -486,6 +486,93 @@ describe('Operators', () => { ] expect(latestMessage.collection.getInner()).toEqual(expectedResult) + + // --- Add a new record --- + input.sendData( + v([3, 0]), + new MultiSet([ + [{ category: 'A', amount: 25 }, 1], + [{ category: 'C', amount: 50 }, 1], + ]), + ) + input.sendFrontier(new Antichain([v([4, 0])])) + graph.run() + + const expectedAddResult = [ + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 25, + }, + ], + 1, + ], + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 20, + }, + ], + -1, + ], + [ + [ + '{"category":"C"}', + { + category: 'C', + minimum: 50, + maximum: 50, + }, + ], + 1, + ], + ] + + expect(latestMessage.collection.getInner()).toEqual(expectedAddResult) + + // --- Delete the current min and max --- + input.sendData( + v([5, 0]), + new MultiSet([ + [{ category: 'A', amount: 5 }, -1], + [{ category: 'A', amount: 25 }, -1], + ]), + ) + input.sendFrontier(new Antichain([v([6, 0])])) + graph.run() + + const expectedDeleteResult = [ + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 10, + maximum: 20, + }, + ], + 1, + ], + [ + [ + '{"category":"A"}', + { + category: 'A', + minimum: 5, + maximum: 25, + }, + ], + -1, + ], + ] + + expect(latestMessage.collection.getInner()).toEqual(expectedDeleteResult) }) test('with median and mode aggregates', () => {