Skip to content

fix: make min and max aggregates ignore retracted values - #90

Open
cpruijsen wants to merge 1 commit into
electric-sql:mainfrom
cpruijsen:fix/issue-88
Open

fix: make min and max aggregates ignore retracted values#90
cpruijsen wants to merge 1 commit into
electric-sql:mainfrom
cpruijsen:fix/issue-88

Conversation

@cpruijsen

Copy link
Copy Markdown

groupBy min and max now net each value's multiplicity and only consider values that are still present (multiplicity > 0), the same rule avg / mode and examples/min_max.ts already use. min and max previously ignored multiplicity, so a retracted extreme stayed in the result. Same change in @electric-sql/d2ts and @electric-sql/d2mini. SQLite groupBy re-exports these functions, so it picks up the fix with no extra copy.

Fixes #88

Decision

Consolidate by value, then min/max over net-positive multiplicities. The alternative is to throw when any value has negative net multiplicity, as MultiSet.min / MultiSet.max do. A streaming retract is a normal -1 in this operator, not an illegal collection; throwing would break the path the issue describes. Can switch to the MultiSet semantics if you want that instead.

Test plan

  • packages/d2ts tests/operators/groupBy.test.ts, with min and max aggregates: category A is {10, 20, 5}, then +25, then -5 / -25minimum: 10, maximum: 20.
  • packages/d2mini same test (delta order is retract-then-insert; values must change, not only emit).
  • packages/d2ts tests/operators-sqlite/groupBy.test.ts (same assertions; uses the re-exported min / max; needs Node 20 / better-sqlite3 native bindings).
  • Optional: two copies of the current max, remove one, max unchanged (net multiplicity stays positive).

Retracted extremes stayed in the result because min and max ignored
multiplicity. Net each value first and only consider those still present.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

min and max aggregate functions ignore multiplicity

1 participant