HermanZeng opened a new issue, #3568: URL: https://github.com/apache/kvrocks/issues/3568
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/kvrocks/issues) and found no similar issues. ### Motivation Redis 8.8 added the `COUNT` aggregation method to the following sorted-set commands: - `ZUNION` - `ZINTER` - `ZUNIONSTORE` - `ZINTERSTORE` The relevant syntax is: ```text ZUNION numkeys key [key ...] [WEIGHTS weight [weight ...]] [AGGREGATE <SUM | MIN | MAX | COUNT>] [WITHSCORES] ZINTER numkeys key [key ...] [WEIGHTS weight [weight ...]] [AGGREGATE <SUM | MIN | MAX | COUNT>] [WITHSCORES] ``` The store variants support the same aggregation methods while writing the result to a destination key. Kvrocks currently recognizes `SUM`, `MIN`, and `MAX`, but rejects `COUNT`. Supporting this option would improve compatibility with Redis 8.8. #### Redis `COUNT` semantics `COUNT` is an aggregation method; it does not limit the number of returned members. For every member in the result: ```text COUNT score(member) = sum of the weights of every input-key occurrence containing that member ``` If `WEIGHTS` is not specified, every input has an implicit weight of `1`. More formally: ```text contribution(i, member) = weight[i], if member exists in input key i 0, otherwise ``` The original member scores are ignored. This is different from the existing aggregation methods: ```text SUM / MIN / MAX contribution = original member score * input weight COUNT contribution = input weight only ``` The membership operation itself remains unchanged: - `ZUNION` includes a member if it exists in at least one input. - `ZINTER` includes a member only if it exists in every input. - Consequently, without explicit weights, every member returned by `ZINTER ... AGGREGATE COUNT` has a score equal to `numkeys`. - With weights, every member returned by the intersection has a score equal to the sum of all input weights. Each input-key argument contributes independently. If the same key is supplied more than once, each occurrence contributes separately using the weight at that input position. #### Example ```text ZADD count:zset1 10 alice 20 bob ZADD count:zset2 100 alice 200 carol ``` Although the original scores differ, they are ignored by `COUNT`: ```text ZUNION 2 count:zset1 count:zset2 AGGREGATE COUNT WITHSCORES ``` The resulting member-to-score mapping is: | Member | Score | Reason | | --- | ---: | --- | | `alice` | 2 | Present in both inputs | | `bob` | 1 | Present only in `count:zset1` | | `carol` | 1 | Present only in `count:zset2` | With non-uniform weights: ```text ZUNION 2 count:zset1 count:zset2 WEIGHTS 2 3 AGGREGATE COUNT WITHSCORES ``` the resulting mapping becomes: | Member | Score | Reason | | --- | ---: | --- | | `alice` | 5 | Contributions `2 + 3` | | `bob` | 2 | Contribution from the first input | | `carol` | 3 | Contribution from the second input | For the corresponding intersection, only `alice` is returned, with score `2` without weights and score `5` with `WEIGHTS 2 3`. This aggregation method can be useful for membership frequency, weighted voting, consensus scoring, recommendation aggregation, and similar ranking or analytics workloads. ### Solution I would like to propose extending the existing sorted-set union/intersection implementation to support `AGGREGATE COUNT` for all four commands. Based on the current shared implementation, one possible approach is: 1. Add `COUNT` to the internal aggregation-method representation. 2. Extend the parser shared by `ZUNION`/`ZINTER` and the parser shared by `ZUNIONSTORE`/`ZINTERSTORE` to recognize `COUNT`. 3. For `COUNT`, use the input weight as the member's contribution instead of multiplying the original member score by the weight. 4. Use that contribution both when a member is first inserted into the intermediate result and when later occurrences are accumulated. 5. Reuse the existing union/intersection membership selection, result construction, and store paths. The fourth point is important because the first occurrence is initialized before subsequent aggregation. Handling `COUNT` only in the later accumulation branch would incorrectly retain the first input's original member score. This approach should keep the existing asymptotic complexity. No storage-format or command-registration changes are expected. I am also happy to adjust the implementation approach based on maintainer feedback. #### Proposed test strategy I propose adding both focused C++ unit tests and Go integration tests so that the internal calculation and external command behavior are verified independently. ##### C++ unit tests Add focused GoogleTest cases in `tests/cppunit/types/zset_test.cc` that directly exercise `ZSet::Union` and `ZSet::Inter` with `COUNT`. The C++ tests would cover: - unweighted union and intersection; - non-uniform weighted union and intersection; - original source scores being ignored; - correct initialization from the first input and accumulation from later inputs; - members occurring in one, some, or all inputs; - duplicate input-key arguments contributing once per input position. ##### Go integration tests Extend `tests/gocase/unit/type/zset/zset_test.go` to verify the command layer and stored results. The Go tests would cover: - `ZUNION`, `ZINTER`, `ZUNIONSTORE`, and `ZINTERSTORE`; - `COUNT` with and without `WEIGHTS`; - `WITHSCORES` for the non-store commands; - cardinality and stored scores for the store commands; - case-insensitive parsing of `COUNT`; - missing input keys and wrong-type inputs; - rejection of an invalid aggregation method; - regression coverage confirming that the default `SUM` behavior and the existing `SUM`, `MIN`, and `MAX` methods remain unchanged. The added tests should exercise every new `COUNT`-specific branch and changed line. The existing C++ and Go test suites should continue to pass. #### Scope This proposal is limited to adding the new aggregation method to the existing four command paths. It does not propose a broader refactor of sorted-set union/intersection or any behavior change to `SUM`, `MIN`, or `MAX`. #### References - [Redis 8.8 release notes](https://redis.io/docs/latest/develop/whats-new/8-8/) - [Redis 8.8 announcement](https://redis.io/blog/announcing-redis-8-8/#sorted-sets-union-and-intersection---count-aggregator) - [`ZUNION` documentation](https://redis.io/docs/latest/commands/zunion/) - [`ZINTER` documentation](https://redis.io/docs/latest/commands/zinter/) - [`ZUNIONSTORE` documentation](https://redis.io/docs/latest/commands/zunionstore/) - [`ZINTERSTORE` documentation](https://redis.io/docs/latest/commands/zinterstore/) If the maintainers agree that this scope is appropriate, I would be happy to implement it and add both focused C++ unit tests and Go integration tests. I would also appreciate any feedback on the proposed approach. Thank you! ### Are you willing to submit a PR? - [x] I'm willing to submit a PR! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
