Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module commands to be marked as unstable and require clients to opt in #2766

Closed
wants to merge 72 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
3892f92
HSCAN VALUES support (v5)
sjpotter May 15, 2024
54d2c0d
enable module commands to be marked as unstable and require clients t…
sjpotter May 26, 2024
5806f68
update all search commands to v5 / unstable interface
sjpotter May 28, 2024
0f2e8d5
update architecture to how we throw an error.
sjpotter May 30, 2024
f8a300f
revert changes needed for resp3 erroring with multi, as now handled b…
sjpotter May 30, 2024
1c77ab0
bring all modules / all commands into v5
sjpotter Jun 2, 2024
f13f47a
leibele's type changing and cleaning up disabling of type mapping
sjpotter Jun 3, 2024
5daf11b
move tuples to map to requiring simple string for map key
sjpotter Jun 3, 2024
a91dded
missed on removal of unstable flag
sjpotter Jun 3, 2024
9baec20
reove ignoreTypeMapping per offline discussion
sjpotter Jun 5, 2024
33adc6f
revert 2 buffer/simple string changes from experimentation
sjpotter Jun 5, 2024
8e7f11c
make resp2/resp3 have same return value
sjpotter Jun 5, 2024
465911e
cleanup
leibale Jun 6, 2024
f08e94f
nit
leibale Jun 25, 2024
6e60233
address leibele's comments + more time series stuff I found in process
sjpotter Jun 26, 2024
6c34d6b
more work on review comments
sjpotter Jun 27, 2024
d896b09
attempt to implement leibele's suggestion to clean up code
sjpotter Jul 8, 2024
7327f5e
wip - changes of personal review
sjpotter Jul 10, 2024
9a85b10
more cleanup
sjpotter Jul 17, 2024
f555c4d
add command/command info and xread/xreadgroup to v5 support
sjpotter Aug 11, 2024
17e4dd5
update tests to actually work right
sjpotter Aug 11, 2024
629e790
hscan values isn't part of 7.4
sjpotter Aug 11, 2024
418f79e
add hash field expiration commands ported over from v4
sjpotter Aug 11, 2024
9dde283
Merge remote-tracking branch 'upstream/v5' into v5-search-broken
sjpotter Aug 14, 2024
4dea5cb
revert cluster broadcasting todo
sjpotter Aug 14, 2024
4f74a86
update test.yaml to have 7.4
sjpotter Aug 15, 2024
f0af1eb
redo time series commands to work with resp3 correctly
sjpotter Aug 15, 2024
4bee30f
unwrap json.type in resp3 from extra array
sjpotter Aug 15, 2024
77592d2
add comment to json.type for what we are doing
sjpotter Aug 19, 2024
60499fc
update time series for resp3 to work consistently
sjpotter Aug 19, 2024
2ac0964
move bloom* info commands to not error out if map type mapping is used
sjpotter Aug 19, 2024
aef901e
tweak unstable search module flag name + add proper error message
sjpotter Aug 19, 2024
60fd22c
have topk info retur a DoubleReply in resp3
sjpotter Aug 19, 2024
4066097
fix xread/xreadgroup for map->array type mapping
sjpotter Aug 19, 2024
2b2bbb5
handle xread/xreadgroup with typing.
sjpotter Aug 19, 2024
9ac2ee8
redo resp2 "map" types to return Objects to be resp3 default compatible
sjpotter Aug 22, 2024
0337343
adding "type mapping" to transformReply
sjpotter Aug 25, 2024
f3013f1
move all 'DoubleReply' in resp2 to new type mapping system
sjpotter Aug 25, 2024
11bd5d2
fix bug + tests
sjpotter Aug 25, 2024
142dfb4
change Buffer('a') to Syumbol() to force TypeError
sjpotter Aug 25, 2024
d42fd70
remove some (unnecessary) jsdoc
leibale Sep 2, 2024
78f1de6
revert and comment out "connect, ready and end events" test
leibale Sep 3, 2024
5676a72
use redis CE docker images for all tests without any rebuilding
sjpotter Sep 23, 2024
e57ca82
move info commands to discussed format
sjpotter Sep 25, 2024
bf8a93c
Merge remote-tracking branch 'origin/v5-search-broken' into v5-search…
sjpotter Sep 25, 2024
52bd5c0
fix bloom info command update
sjpotter Sep 25, 2024
3d0485b
fix cms.info test
sjpotter Sep 25, 2024
9adac9d
rename unstableResp3SearchModule cofnig + xread/group support
sjpotter Sep 29, 2024
ea51f39
revert to be v4 compat for xread/xreadgroup
sjpotter Oct 6, 2024
5202955
make versions only use latest release
sjpotter Oct 7, 2024
b4b67f6
Merge branch 'v5-search-broken' into change-testing
sjpotter Oct 7, 2024
58f88b5
Merge pull request #1 from sjpotter/change-testing
sjpotter Oct 7, 2024
8ad4171
run tests against PRs against v5 branch
sjpotter Oct 7, 2024
d0396e7
fixups for testing
sjpotter Oct 7, 2024
faa67f3
fix running docker with redis-stack
sjpotter Oct 7, 2024
830d5db
attempt at fixing 1 github test issue
sjpotter Oct 7, 2024
fcdb1c0
fix hello test
sjpotter Oct 7, 2024
7ead8e4
fix `TS.M[REV]RANGE[_WITHLABELS|_SELECTED_LABELS]` & `TS.MGET[_WITHLA…
leibale Oct 7, 2024
68ca83b
fix time series
sjpotter Oct 7, 2024
9e8c183
fix non profile search commands
sjpotter Oct 7, 2024
3f65936
attempt to make ts info / info debug backwards cmmpatable
sjpotter Oct 8, 2024
839e14b
update to handle 'warning' in profile data return
sjpotter Oct 8, 2024
91ed69e
attempt to make test more reliable
sjpotter Oct 8, 2024
2d18b36
fix hello test to test if modules is an array
sjpotter Oct 9, 2024
31d61b8
clean up rejection fix for clientkill
sjpotter Oct 9, 2024
25fdd32
fix ts.info/debug again to hopefully be cleaner
sjpotter Oct 9, 2024
59a27cc
small cleanups
sjpotter Oct 9, 2024
bf11c35
more cleanup
sjpotter Oct 9, 2024
1a5ac15
transformTuplesReply can be a BlobStringReply or SimpleStringReply
sjpotter Oct 9, 2024
16950d0
update ft.info to actually work + with newest options
sjpotter Oct 9, 2024
e1b935c
small tweak for stopWords
sjpotter Oct 9, 2024
1f346f8
Merge remote-tracking branch 'upstream/v5' into v5-search-broken
sjpotter Oct 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 61 additions & 3 deletions packages/bloom/lib/commands/bloom/INFO.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,69 @@
import { RedisArgument, Command } from '@redis/client/dist/lib/RESP/types';
import { RedisArgument, Command, UnwrapReply, NullReply, NumberReply, TuplesToMapReply, Resp2Reply, SimpleStringReply } from '@redis/client/dist/lib/RESP/types';

export type BfInfoReplyMap = TuplesToMapReply<[
[SimpleStringReply<'Capacity'>, NumberReply],
[SimpleStringReply<'Size'>, NumberReply],
[SimpleStringReply<'Number of filters'>, NumberReply],
[SimpleStringReply<'Number of items inserted'>, NumberReply],
[SimpleStringReply<'Expansion rate'>, NullReply | NumberReply]
]>;

export interface BfInfoReply {
capacity?: NumberReply;
size?: NumberReply;
numberOfFilters?: NumberReply;
numberOfInsertedItems?: NumberReply;
expansionRate?: NullReply | NumberReply;
}

export default {
FIRST_KEY_INDEX: 1,
IS_READ_ONLY: true,
transformArguments(key: RedisArgument) {
return ['BF.INFO', key];
},
// TODO
transformReply: undefined as unknown as () => any
transformReply: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/redis/node-redis/blob/v5/docs/v4-to-v5.md?plain=1#L238
#2506

I'm not sure if we should keep the keys as is for the sake of consistency or transform them for the sake of "friendly/javascripty"..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it says there "to avoid unneccessary transformations" (and confusion but I honestly don't think that is the case).

In this case a transformation is neccessary anyways, so might as well make it friendly. I also distinguish between "info" objects that are defined data structures, and generic maps that can contain any set of keys/values (as I'll discuss on the next one)

2(reply: UnwrapReply<Resp2Reply<BfInfoReplyMap>>): BfInfoReply {
return {
capacity: reply[1],
size: reply[3],
numberOfFilters: reply[5],
numberOfInsertedItems: reply[7],
expansionRate: reply[9]
};
},
3(reply: UnwrapReply<BfInfoReplyMap>): BfInfoReply {
if (reply instanceof Map) {
throw new Error("BF.INFO shouldn't be used with type mapping to map or array");
/*
return {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only modify the keys ('Number of items inserted' -> 'numberOfInsertedItems'), not the data structure (Map -> Array)

edit: same for some more info commands, I'm not going to copy this comment..

capacity: reply.get("Capacity" as unknown as BlobStringReply<'Capacity'>),
size: reply.get("Size" as unknown as BlobStringReply<"Size">),
numberOfFilters: reply.get("Number of filters" as unknown as BlobStringReply<"Number of filters">),
numberOfInsertedItems: reply.get('Number of items inserted' as unknown as BlobStringReply<'Number of items inserted'>),
expansionRate: reply.get('Expansion rate' as unknown as BlobStringReply<'Expansion rate'>),
}
*/
} else if (reply instanceof Array) {
throw new Error("BF.INFO shouldn't be used with type mapping to map or array");
/*
return {
capacity: reply[1],
size: reply[3],
numberOfFilters: reply[5],
numberOfInsertedItems: reply[7],
expansionRate: reply[9]
}
*/
} else {
return {
capacity: reply["Capacity"],
size: reply["Size"],
numberOfFilters: reply["Number of filters"],
numberOfInsertedItems: reply["Number of items inserted"],
expansionRate: reply["Expansion rate"]
};
}
}
}
} as const satisfies Command;
56 changes: 45 additions & 11 deletions packages/bloom/lib/commands/count-min-sketch/INFO.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { RedisArgument, TuplesToMapReply, BlobStringReply, NumberReply, UnwrapReply, Resp2Reply, Command } from '@redis/client/dist/lib/RESP/types';
import { RedisArgument, TuplesToMapReply, NumberReply, UnwrapReply, Resp2Reply, Command, SimpleStringReply } from '@redis/client/dist/lib/RESP/types';

export type BfInfoReply = TuplesToMapReply<[
[BlobStringReply<'width'>, NumberReply],
[BlobStringReply<'depth'>, NumberReply],
[BlobStringReply<'count'>, NumberReply]
export type CmsInfoReplyMap = TuplesToMapReply<[
[SimpleStringReply<'width'>, NumberReply],
[SimpleStringReply<'depth'>, NumberReply],
[SimpleStringReply<'count'>, NumberReply]
]>;

export interface CmsInfoReply {
width?: NumberReply;
depth?: NumberReply;
count?: NumberReply;
}

export default {
FIRST_KEY_INDEX: 1,
Expand All @@ -13,11 +19,39 @@ export default {
return ['CMS.INFO', key];
},
transformReply: {
2: (reply: UnwrapReply<Resp2Reply<BfInfoReply>>) => ({
width: reply[1],
depth: reply[3],
count: reply[5]
}),
3: undefined as unknown as () => BfInfoReply
2(reply: UnwrapReply<Resp2Reply<CmsInfoReplyMap>>): CmsInfoReply {
return {
width: reply[1],
depth: reply[3],
count: reply[5]
};
},
3(reply: UnwrapReply<CmsInfoReplyMap>): CmsInfoReply {
if (reply instanceof Map) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case we should just return the reply as is, as we don't modify the keys nor the values

edit: same for some more info commands, I'm not going to copy this comment..

throw new Error("CMS.INFO shouldn't be used with type mapping to map or array");
/*
return {
width: reply.get("width" as unknown as BlobStringReply<'width'>),
depth: reply.get("depth" as unknown as BlobStringReply<"depth">),
count: reply.get("count" as unknown as BlobStringReply<"count">)
}
*/
} else if (reply instanceof Array) {
throw new Error("CMS.INFO shouldn't be used with type mapping to map or array");
/*
return {
width: reply[1],
depth: reply[3],
count: reply[5]
}
*/
} else {
return {
width: reply['width'],
depth: reply['depth'],
count: reply['count']
};
}
}
}
} as const satisfies Command;
124 changes: 79 additions & 45 deletions packages/bloom/lib/commands/cuckoo/INFO.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,87 @@
import { RedisArgument, Command } from '@redis/client/dist/lib/RESP/types';
import { RedisArgument, Command, NumberReply, TuplesToMapReply, UnwrapReply, Resp2Reply, SimpleStringReply } from '@redis/client/dist/lib/RESP/types';

export type CfInfoReplyMap = TuplesToMapReply<[
[SimpleStringReply<'Size'>, NumberReply],
[SimpleStringReply<'Number of buckets'>, NumberReply],
[SimpleStringReply<'Number of filters'>, NumberReply],
[SimpleStringReply<'Number of items inserted'>, NumberReply],
[SimpleStringReply<'Number of items deleted'>, NumberReply],
[SimpleStringReply<'Bucket size'>, NumberReply],
[SimpleStringReply<'Expansion rate'>, NumberReply],
[SimpleStringReply<'Max iterations'>, NumberReply]
]>;

export interface CfInfoReply {
size: NumberReply;
numberOfBuckets: NumberReply;
numberOfFilters: NumberReply;
numberOfInsertedItems: NumberReply;
numberOfDeletedItems: NumberReply;
bucketSize: NumberReply;
expansionRate: NumberReply;
maxIteration: NumberReply;
}

export default {
FIRST_KEY_INDEX: 1,
IS_READ_ONLY: true,
transformArguments(key: RedisArgument) {
return ['CF.INFO', key];
},
// TODO
// export type InfoRawReply = [
// _: string,
// size: number,
// _: string,
// numberOfBuckets: number,
// _: string,
// numberOfFilters: number,
// _: string,
// numberOfInsertedItems: number,
// _: string,
// numberOfDeletedItems: number,
// _: string,
// bucketSize: number,
// _: string,
// expansionRate: number,
// _: string,
// maxIteration: number
// ];

// export interface InfoReply {
// size: number;
// numberOfBuckets: number;
// numberOfFilters: number;
// numberOfInsertedItems: number;
// numberOfDeletedItems: number;
// bucketSize: number;
// expansionRate: number;
// maxIteration: number;
// }

// export function transformReply(reply: InfoRawReply): InfoReply {
// return {
// size: reply[1],
// numberOfBuckets: reply[3],
// numberOfFilters: reply[5],
// numberOfInsertedItems: reply[7],
// numberOfDeletedItems: reply[9],
// bucketSize: reply[11],
// expansionRate: reply[13],
// maxIteration: reply[15]
// };
// }
transformReply: undefined as unknown as () => any
transformReply: {
2(reply: UnwrapReply<Resp2Reply<CfInfoReplyMap>>): CfInfoReply {
return {
size: reply[1],
numberOfBuckets: reply[3],
numberOfFilters: reply[5],
numberOfInsertedItems: reply[7],
numberOfDeletedItems: reply[9],
bucketSize: reply[11],
expansionRate: reply[13],
maxIteration: reply[15]
};
},
3: (reply: UnwrapReply<CfInfoReplyMap>): CfInfoReply => {
if (reply instanceof Map) {
throw new Error("CF.INFO shouldn't be used with type mapping to map or array");
/*
return {
size: reply.get("Size" as unknown as BlobStringReply<"Size">)!,
numberOfBuckets: reply.get('Number of buckets' as unknown as BlobStringReply<'Number of buckets'>)!,
numberOfFilters: reply.get('Number of filters' as unknown as BlobStringReply<"Number of filters">)!,
numberOfInsertedItems: reply.get('Number of items inserted' as unknown as BlobStringReply<'Number of items inserted'>)!,
numberOfDeletedItems: reply.get('Number of items deleted' as unknown as BlobStringReply<'Number of items deleted'>)!,
bucketSize: reply.get('Bucket size' as unknown as BlobStringReply<'Bucket size'>)!,
expansionRate: reply.get('Expansion rate' as unknown as BlobStringReply<'Expansion rate'>)!,
maxIteration: reply.get('Max iterations' as unknown as BlobStringReply<'Max iterations'>)!
}
*/
} else if (reply instanceof Array) {
throw new Error("CF.INFO shouldn't be used with type mapping to map or array");
/*
return {
size: reply[1],
numberOfBuckets: reply[3],
numberOfFilters: reply[5],
numberOfInsertedItems: reply[7],
numberOfDeletedItems: reply[9],
bucketSize: reply[11],
expansionRate: reply[13],
maxIteration: reply[15]
}
*/
} else {
return {
size: reply['Size'],
numberOfBuckets: reply['Number of buckets'],
numberOfFilters: reply['Number of filters'],
numberOfInsertedItems: reply['Number of items inserted'],
numberOfDeletedItems: reply['Number of items deleted'],
bucketSize: reply['Bucket size'],
expansionRate: reply['Expansion rate'],
maxIteration: reply['Max iterations']
};
}
}
}
} as const satisfies Command;
Loading