Add ZINTER/ZUNION command by yangbodong22011 · Pull Request #7794 · redis/redis

yangbodong22011 · 2020-09-14T13:42:02Z

Syntax:

ZINTER/ZUNION numkeys key [key ...] [WEIGHTS weight [weight ...]]
[AGGREGATE SUM|MIN|MAX] [WITHSCORES]

see #7624

src/t_zset.c

src/db.c

src/server.c

src/t_zset.c

tests/unit/type/zset.tcl

src/t_zset.c

src/server.c

oranagra · 2020-09-16T06:46:26Z

@redis/core-team please approve.
@yangbodong22011 can you please also make a PR for redis-doc?

yangbodong22011 · 2020-09-16T06:52:28Z

@yangbodong22011 can you please also make a PR for redis-doc?

sure, I will do it.

madolson

Just a minor comment.

src/db.c

Syntax: ZINTER/ZUNION numkeys key [key ...] [WEIGHTS weight [weight ...]] [AGGREGATE SUM|MIN|MAX] [WITHSCORES] see redis#7624

oranagra

having some issues with github.
please squash this into the second commit:

-     * reply with syntax error. */
-    if (num < 1 || num > (argc - (keyCountOfs+1))/keyStep) {
+     * reply with syntax error (no input keys). */
+    if (num < 1 || num > (argc - firstKeyOfs)/keyStep) {

yangbodong22011 · 2020-09-17T08:59:54Z

@oranagra Our current runtest or runtest-cluster seems to be unable to cover the test for genericGetKeys, if I want to add a test for this part, do you have any suggestions?

oranagra · 2020-09-17T09:23:12Z

@yangbodong22011 i don't have a good idea how to test it.
i can think of 2 options:

enable ACL key name rules or cluster, and run the normal zset tests (tests the happy path).
write some fuzzer that uses COMMAND command info to try and generate random commands who have some chance of succeeding (passing the arity check), and uses COMMAND GETKEYS (just to check that there are no crashes or leaks).

Either of these is not a candidate for the current PR, if you can think of something simple we can add it now, if not, i invite you to do some manual test and leave that topic for another PR.

yangbodong22011 · 2020-09-17T11:07:21Z

Either of these is not a candidate for the current PR, if you can think of something simple we can add it now, if not, i invite you to do some manual test and leave that topic for another PR.

I have completed the manual test and found no problems, In addition, thank you for your suggestion, I will consider future PR.

oranagra · 2020-09-23T07:07:26Z

@redis/core-team reminder. please approve new commands.

see: redis/redis#7794

soloestoy · 2020-09-24T03:56:07Z

good job! @yangbodong22011

oranagra · 2020-09-24T06:00:07Z

merged. thank you!.
@yangbodong22011 please don't forget to do the redis-doc pr.

yangbodong22011 · 2020-09-24T06:10:44Z

please don't forget to do the redis-doc pr.

PR has been submitted, redis/redis-doc#1396

**Bug Fixes:** * [redis#7219](RediSearch/RediSearch#7219) Fix a concurrency issue on Reducer in `FT.AGGREGATE` * [redis#7255](RediSearch/RediSearch#7255) Fix `BM25STD` underflow wraparound * [redis#7275](RediSearch/RediSearch#7275) Report used memory as `unsigned long long` to avoid underflows * [redis#7264](RediSearch/RediSearch#7264) Fix `totalDocsLen` updates * [redis#6995](RediSearch/RediSearch#6995) Do not fanout `FT.INFO` to replicas * [redis#7350](RediSearch/RediSearch#7350) Fix `FT.CREATE` failure with LeanVec parameters on non-Intel architectures * [redis#7384](RediSearch/RediSearch#7384) Fix index load from RDB temporary memory overhead * [redis#7459](RediSearch/RediSearch#7459) Fix Fork GC potential double-free on error path * [redis#7458](RediSearch/RediSearch#7458) Fix a GC performence regression * [redis#7470](RediSearch/RediSearch#7470) Avoid draining worker thread pool from FLUSH callback to avoid deadlocks * [redis#7554](RediSearch/RediSearch#7554) Handle the case where `SCORE` is sent alone without extra fields (coordinator) * [redis#7685](RediSearch/RediSearch#7685) Fix cursor logical leak * [redis#7794](RediSearch/RediSearch#7794) Fix `cmp_strings()` to correctly handle binary data with embedded NULLs in TOLIST reducer in FT.AGGREGATE * [redis#7873](RediSearch/RediSearch#7873) Handle warnings in empty `FT.AGGREGATE` replies (cluster) * [redis#7886](RediSearch/RediSearch#7886) Remove non-TEXT fields from the spec keys dictionary * [redis#7904](RediSearch/RediSearch#7904) Refactor keys dictionary handling * [redis#7901](RediSearch/RediSearch#7901) Support multiple warnings in reply * [redis#8083](RediSearch/RediSearch#8083) Fix incorrect FULLTEXT field metric counts * [redis#8153](RediSearch/RediSearch#8153) Fix configuration registration issues **Improvements:** * [redis#7154](RediSearch/RediSearch#7154) `FT.AGGREGATE` can return Background Indexing OOM warnings * [redis#7083](RediSearch/RediSearch#7083) Add the default text scorer as a configuration option * [redis#7341](RediSearch/RediSearch#7341) Rename `FT.PROFILE` counter fields * [redis#7436](RediSearch/RediSearch#7436) Enhance `FT.PROFILE` with vector search execution details * [redis#7435](RediSearch/RediSearch#7435) Ensure full `FT.PROFILE` output on timeout with RETURN policy * [redis#7534](RediSearch/RediSearch#7534) Reduce the number of worker threads asynchronously to avoid deadlocks during queries * [redis#7614](RediSearch/RediSearch#7614) Track timeout warnings and errors in INFO * [redis#7646](RediSearch/RediSearch#7646) Track `maxprefixexpansions` warnings and errors in INFO * [redis#7577](RediSearch/RediSearch#7577) Track query syntax/argument errors (basis for query error tracking) * [redis#7737](RediSearch/RediSearch#7737) Add `Internal cursor reads` metric to cluster `FT.PROFILE` output * [redis#7759](RediSearch/RediSearch#7759) Extend indexing metrics * [redis#7710](RediSearch/RediSearch#7710) Support `WITHCOUNT` keyword in `FT.AGGREGATE` * [redis#7957](RediSearch/RediSearch#7957) Persist query warnings across cursor reads * [redis#8054](RediSearch/RediSearch#8054) Add logging for index-related commands * [redis#8151](RediSearch/RediSearch#8151) Fix shard total profile time reporting in `FT.PROFILE` * [redis#8103](RediSearch/RediSearch#8103) Output current thread IndexSpec information on crash

**Bug Fixes:** * [#7219](RediSearch/RediSearch#7219) Fix a concurrency issue on Reducer in `FT.AGGREGATE` * [#7255](RediSearch/RediSearch#7255) Fix `BM25STD` underflow wraparound * [#7275](RediSearch/RediSearch#7275) Report used memory as `unsigned long long` to avoid underflows * [#7264](RediSearch/RediSearch#7264) Fix `totalDocsLen` updates * [#6995](RediSearch/RediSearch#6995) Do not fanout `FT.INFO` to replicas * [#7350](RediSearch/RediSearch#7350) Fix `FT.CREATE` failure with LeanVec parameters on non-Intel architectures * [#7694](RediSearch/RediSearch#7694) Use asynchronous jobs for GC in SVS to accelerate execution * [#7384](RediSearch/RediSearch#7384) Fix index load from RDB temporary memory overhead * [#7459](RediSearch/RediSearch#7459) Fix Fork GC potential double-free on error path * [#7458](RediSearch/RediSearch#7458) Fix a GC performence regression * [#7470](RediSearch/RediSearch#7470) Avoid draining worker thread pool from FLUSH callback to avoid deadlocks * [#7554](RediSearch/RediSearch#7554) Handle the case where `SCORE` is sent alone without extra fields (coordinator) * [#7685](RediSearch/RediSearch#7685) Fix cursor logical leak * [#7794](RediSearch/RediSearch#7794) Fix `cmp_strings()` to correctly handle binary data with embedded NULLs in TOLIST reducer in FT.AGGREGATE * [#7873](RediSearch/RediSearch#7873) Handle warnings in empty `FT.AGGREGATE` replies (cluster) * [#7886](RediSearch/RediSearch#7886) Remove non-TEXT fields from the spec keys dictionary * [#7904](RediSearch/RediSearch#7904) Refactor keys dictionary handling * [#7901](RediSearch/RediSearch#7901) Support multiple warnings in reply * [#8083](RediSearch/RediSearch#8083) Fix incorrect FULLTEXT field metric counts * [#8153](RediSearch/RediSearch#8153) Fix configuration registration issues **Improvements:** * [#7154](RediSearch/RediSearch#7154) `FT.AGGREGATE` can return Background Indexing OOM warnings * [#7083](RediSearch/RediSearch#7083) Add the default text scorer as a configuration option * [#7341](RediSearch/RediSearch#7341) Rename `FT.PROFILE` counter fields * [#7436](RediSearch/RediSearch#7436) Enhance `FT.PROFILE` with vector search execution details * [#7435](RediSearch/RediSearch#7435) Ensure full `FT.PROFILE` output on timeout with RETURN policy * [#7534](RediSearch/RediSearch#7534) Reduce the number of worker threads asynchronously to avoid deadlocks during queries * [#7614](RediSearch/RediSearch#7614) Track timeout warnings and errors in INFO * [#7646](RediSearch/RediSearch#7646) Track `maxprefixexpansions` warnings and errors in INFO * [#7577](RediSearch/RediSearch#7577) Track query syntax/argument errors (basis for query error tracking) * [#7737](RediSearch/RediSearch#7737) Add `Internal cursor reads` metric to cluster `FT.PROFILE` output * [#7759](RediSearch/RediSearch#7759) Extend indexing metrics * [#7710](RediSearch/RediSearch#7710) Support `WITHCOUNT` keyword in `FT.AGGREGATE` * [#7957](RediSearch/RediSearch#7957) Persist query warnings across cursor reads * [#8054](RediSearch/RediSearch#8054) Add logging for index-related commands * [#8151](RediSearch/RediSearch#8151) Fix shard total profile time reporting in `FT.PROFILE` * [#8103](RediSearch/RediSearch#8103) Output current thread IndexSpec information on crash

yangbodong22011 commented Sep 14, 2020

View reviewed changes

src/t_zset.c Outdated Show resolved Hide resolved

oranagra reviewed Sep 15, 2020

View reviewed changes

src/db.c Outdated Show resolved Hide resolved

src/server.c Outdated Show resolved Hide resolved

src/t_zset.c Outdated Show resolved Hide resolved

src/t_zset.c Outdated Show resolved Hide resolved

tests/unit/type/zset.tcl Outdated Show resolved Hide resolved

oranagra added state:major-decision Requires core team consensus state:needs-doc-pr requires a PR to redis-doc repository labels Sep 15, 2020

oranagra added this to the Redis 6.2 milestone Sep 15, 2020

oranagra linked an issue Sep 15, 2020 that may be closed by this pull request

[NEW] ZINTER/ZUNION support #7624

Closed

yangbodong22011 force-pushed the feature-zinter-zunion-command branch from 56210e6 to daafcbf Compare September 15, 2020 15:10

oranagra reviewed Sep 15, 2020

View reviewed changes

src/t_zset.c Outdated Show resolved Hide resolved

src/server.c Outdated Show resolved Hide resolved

yangbodong22011 force-pushed the feature-zinter-zunion-command branch from daafcbf to 52ecda1 Compare September 16, 2020 02:02

oranagra previously approved these changes Sep 16, 2020

View reviewed changes

oranagra added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label Sep 16, 2020

madolson reviewed Sep 17, 2020

View reviewed changes

src/db.c Outdated Show resolved Hide resolved

Add ZINTER/ZUNION command

5027724

Syntax: ZINTER/ZUNION numkeys key [key ...] [WEIGHTS weight [weight ...]] [AGGREGATE SUM|MIN|MAX] [WITHSCORES] see redis#7624

yangbodong22011 dismissed oranagra’s stale review via 5027724 September 17, 2020 07:54

yangbodong22011 force-pushed the feature-zinter-zunion-command branch from 52ecda1 to 5027724 Compare September 17, 2020 07:54

oranagra reviewed Sep 17, 2020

View reviewed changes

Refactor multi-key command get keys proc

b5e29ec

yangbodong22011 force-pushed the feature-zinter-zunion-command branch from c5e234e to b5e29ec Compare September 17, 2020 08:29

oranagra approved these changes Sep 17, 2020

View reviewed changes

itamarhaber approved these changes Sep 23, 2020

View reviewed changes

yangbodong22011 added a commit to yangbodong22011/redis-doc that referenced this pull request Sep 23, 2020

Add ZINTER/ZUNION command

0d7745e

see: redis/redis#7794

yangbodong22011 mentioned this pull request Sep 23, 2020

Add ZINTER/ZUNION command redis/redis-doc#1396

Merged

soloestoy approved these changes Sep 24, 2020

View reviewed changes

oranagra merged commit b7ce583 into redis:unstable Sep 24, 2020

oranagra mentioned this pull request Sep 29, 2020

A better approach for COMMAND INFO for movablekeys commands #7297

Closed

oranagra removed this from the Next minor backlog milestone Oct 19, 2020

oranagra mentioned this pull request Nov 4, 2020

Add new commands ZDIFF and ZDIFFSTORE #7961

Merged

oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Dec 9, 2020

jiekun mentioned this pull request Dec 21, 2020

Redis 6.2 New Commands Check List redis/redis-py#1434

Closed

18 tasks

oranagra mentioned this pull request Jan 13, 2021

Redis 6.2 RC1. #8187

Merged

Conversation

yangbodong22011 commented Sep 14, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oranagra commented Sep 16, 2020

Uh oh!

yangbodong22011 commented Sep 16, 2020

Uh oh!

madolson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oranagra left a comment

Choose a reason for hiding this comment

Uh oh!

yangbodong22011 commented Sep 17, 2020

Uh oh!

oranagra commented Sep 17, 2020

Uh oh!

yangbodong22011 commented Sep 17, 2020

Uh oh!

oranagra commented Sep 23, 2020

Uh oh!

soloestoy commented Sep 24, 2020

Uh oh!

oranagra commented Sep 24, 2020

Uh oh!

yangbodong22011 commented Sep 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants