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

Node: Enable +/-inf as score for ZADD #3370

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#### Changes
* Core: Add `opentelemetry` protocols support ([#3191](https://github.com/valkey-io/valkey-glide/pull/3191))
* Node: Fix ZADD, enabling `+inf` and `-inf` as score ([#3370](https://github.com/valkey-io/valkey-glide/pull/3370))
* Go: Add JSON.SET and JSON.GET ([#3115](https://github.com/valkey-io/valkey-glide/pull/3115))
* Csharp: updating xUnit, adding xUnit analyser rules and guidelines ([#3035](https://github.com/valkey-io/valkey-glide/pull/3035))
* Go: Add ZRangeStore ([3105](https://github.com/valkey-io/valkey-glide/pull/3105))
Expand Down
4 changes: 4 additions & 0 deletions node/npm/glide/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ function initialize() {
ReturnTypeAttribute,
ReturnTypeJson,
UniversalReturnTypeJson,
Score,
ElementAndScore,
} = nativeBinding;

module.exports = {
Expand Down Expand Up @@ -340,6 +342,8 @@ function initialize() {
ReturnTypeAttribute,
ReturnTypeJson,
UniversalReturnTypeJson,
Score,
ElementAndScore,
};

globalObject = Object.assign(global, nativeBinding);
Expand Down
25 changes: 18 additions & 7 deletions node/src/BaseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ import {
TimeUnit,
ZAddOptions,
ZScanOptions,
convertElementsAndScores,
convertFieldsAndValuesToHashDataType,
convertKeysAndEntries,
createAppend,
Expand Down Expand Up @@ -348,6 +347,22 @@ export type HashDataType = {
*/
export type StreamEntryDataType = Record<string, [GlideString, GlideString][]>;

/**
* Union type that can store either a number or positive/negative infinity.
*/
export type Score = number | "+inf" | "-inf";

/**
* Data type which represents sorted sets data for input parameter of ZADD command,
* including element and its respective score.
*/
export type ElementAndScore = {
/** The sorted set element name. */
element: GlideString;
/** The element score. */
score: Score;
};

/**
* @internal
* Convert `GlideRecord<number>` recevied after resolving the value pointer into `SortedSetDataType`.
Expand Down Expand Up @@ -4020,15 +4035,11 @@ export class BaseClient {
*/
public async zadd(
key: GlideString,
membersAndScores: SortedSetDataType | Record<string, number>,
membersAndScores: ElementAndScore[] | Record<string, Score>,
options?: ZAddOptions,
): Promise<number> {
return this.createWritePromise(
createZAdd(
key,
convertElementsAndScores(membersAndScores),
options,
),
createZAdd(key, membersAndScores, options),
);
}

Expand Down
19 changes: 17 additions & 2 deletions node/src/Commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import {
GlideRecord,
GlideString,
HashDataType,
Score,
ObjectType,
SortedSetDataType,
ElementAndScore,
} from "./BaseClient";
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
import { GlideClient } from "./GlideClient";
Expand Down Expand Up @@ -1442,7 +1444,7 @@ export function convertElementsAndScores(
*/
export function createZAdd(
key: GlideString,
membersAndScores: SortedSetDataType,
membersAndScores: ElementAndScore[] | Record<string, Score>,
options?: ZAddOptions,
incr = false,
): command_request.Command {
Expand Down Expand Up @@ -1476,7 +1478,20 @@ export function createZAdd(
args.push("INCR");
}

membersAndScores.forEach((p) => args.push(p.score.toString(), p.element));
if (Array.isArray(membersAndScores)) {
for (let i = 0, len = membersAndScores.length; i < len; i++) {
const item = membersAndScores[i];
args.push(item.score.toString(), item.element);
}
} else {
const members = Object.keys(membersAndScores);

for (let i = 0, len = members.length; i < len; i++) {
const member = members[i];
args.push(membersAndScores[member].toString(), member);
}
}

return createCommand(RequestType.ZAdd, args);
}

Expand Down
14 changes: 4 additions & 10 deletions node/src/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ import {
GlideString,
HashDataType,
ReadFrom, // eslint-disable-line @typescript-eslint/no-unused-vars
SortedSetDataType,
convertGlideRecord,
Score,
ElementAndScore,
} from "./BaseClient";

import {
Expand Down Expand Up @@ -66,7 +67,6 @@ import {
TimeUnit,
ZAddOptions,
ZScanOptions,
convertElementsAndScores,
convertFieldsAndValuesToHashDataType,
createAppend,
createBLMPop,
Expand Down Expand Up @@ -1738,16 +1738,10 @@ class BaseTransaction<T extends BaseTransaction<T>> {
*/
public zadd(
key: GlideString,
membersAndScores: SortedSetDataType | Record<string, number>,
membersAndScores: ElementAndScore[] | Record<string, Score>,
options?: ZAddOptions,
): T {
return this.addAndReturn(
createZAdd(
key,
convertElementsAndScores(membersAndScores),
options,
),
);
return this.addAndReturn(createZAdd(key, membersAndScores, options));
}

/**
Expand Down
31 changes: 31 additions & 0 deletions node/tests/SharedTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ import {
convertFieldsAndValuesToHashDataType,
convertGlideRecordToRecord,
parseInfoResponse,
Score,
ElementAndScore,
} from "..";
import { ValkeyCluster } from "../../utils/TestUtils";
import { Client, GetAndSetRandomValue, getFirstResult } from "./TestUtilities";
Expand Down Expand Up @@ -4154,6 +4156,17 @@ export function runBaseTests(config: {
expect(
await client.zadd(key, newMembersScores, { changed: true }),
).toEqual(2);
const infMembersScores: Record<string, Score> = {
infMember: "+inf",
negInfMember: "-inf",
};
expect(await client.zadd(key, infMembersScores)).toEqual(2);

const infElementAndScore: ElementAndScore[] = [
{ element: "infMemberEAS", score: "+inf" },
{ element: "negInfMemberEAS", score: "-inf" },
];
expect(await client.zadd(key, infElementAndScore)).toEqual(2);
}, protocol);
},
config.timeout,
Expand Down Expand Up @@ -4537,6 +4550,24 @@ export function runBaseTests(config: {

expect(await client.set(key2, "foo")).toEqual("OK");
await expect(client.zscore(key2, "foo")).rejects.toThrow();

const inf_key = uuidv4();
const infMembersScores: Record<string, Score> = {
infMember: "+inf",
negInfMember: "-inf",
};
expect(await client.zadd(inf_key, infMembersScores)).toEqual(2);
expect(await client.zscore(inf_key, "infMember")).toEqual(
Infinity,
);

const inf_key2 = uuidv4();
expect(
await client.zadd(inf_key2, { infMember: -Infinity }),
).toEqual(1);
expect(await client.zscore(inf_key2, "infMember")).toEqual(
-Infinity,
);
}, protocol);
},
config.timeout,
Expand Down
46 changes: 34 additions & 12 deletions node/tests/TestUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1174,22 +1174,38 @@ export async function transactionTest(
{ element: member3, score: 3.5 },
{ element: member4, score: 4 },
{ element: member5, score: 5 },
{ element: "infMember", score: "+inf" },
{ element: "negInfMember", score: "-inf" },
]);
responseData.push(["zadd(key8, { ... } ", 5]);
responseData.push(["zadd(key8, { ... } ", 7]);
baseTransaction.zrank(key8, member1);
responseData.push(['zrank(key8, "member1")', 0]);
responseData.push(['zrank(key8, "member1")', 1]);
baseTransaction.zrank(key8, "negInfMember");
responseData.push(['zrank(key8, "negInfMember")', 0]);

if (!cluster.checkIfServerVersionLessThan("7.2.0")) {
baseTransaction.zrankWithScore(key8, member1);
responseData.push(['zrankWithScore(key8, "member1")', [0, 1]]);
responseData.push(['zrankWithScore(key8, "member1")', [1, 1]]);
baseTransaction.zrankWithScore(key8, "negInfMember");
responseData.push([
'zrankWithScore(key8, "negInfMember")',
[0, -Infinity],
]);
}

baseTransaction.zrevrank(key8, "member5");
responseData.push(['zrevrank(key8, "member5")', 0]);
responseData.push(['zrevrank(key8, "member5")', 1]);
baseTransaction.zrevrank(key8, "infMember");
responseData.push(['zrevrank(key8, "infMember")', 0]);

if (!cluster.checkIfServerVersionLessThan("7.2.0")) {
baseTransaction.zrevrankWithScore(key8, "member5");
responseData.push(['zrevrankWithScore(key8, "member5")', [0, 5]]);
responseData.push(['zrevrankWithScore(key8, "member5")', [1, 5]]);
baseTransaction.zrevrankWithScore(key8, "infMember");
responseData.push([
'zrevrankWithScore(key8, "infMember")',
[0, Infinity],
]);
}

baseTransaction.zaddIncr(key8, member2, 1);
Expand All @@ -1199,28 +1215,34 @@ export async function transactionTest(
baseTransaction.zrem(key8, [member1]);
responseData.push(['zrem(key8, ["member1"])', 1]);
baseTransaction.zcard(key8);
responseData.push(["zcard(key8)", 4]);
responseData.push(["zcard(key8)", 6]);

baseTransaction.zscore(key8, member2);
responseData.push(['zscore(key8, "member2")', 3.0]);
baseTransaction.zscore(key8, "infMember");
responseData.push(['zscore(key8, "infMember")', Infinity]);
baseTransaction.zrange(key8, { start: 0, end: -1 });
responseData.push([
"zrange(key8, { start: 0, end: -1 })",
[
"negInfMember",
member2.toString(),
member3.toString(),
member4.toString(),
member5.toString(),
"infMember",
],
]);
baseTransaction.zrangeWithScores(key8, { start: 0, end: -1 });
responseData.push([
"zrangeWithScores(key8, { start: 0, end: -1 })",
convertRecordToGlideRecord({
negInfMember: -Infinity,
member2: 3,
member3: 3.5,
member4: 4,
member5: 5,
infMember: Infinity,
}),
]);
baseTransaction.zadd(key12, [
Expand Down Expand Up @@ -1261,7 +1283,7 @@ export async function transactionTest(
baseTransaction.zrangeStore(key8, key8, { start: 0, end: -1 });
responseData.push([
"zrangeStore(key8, key8, { start: 0, end: -1 })",
4,
6,
]);
baseTransaction.zdiff([key13, key12]);
responseData.push(["zdiff([key13, key12])", ["three"]]);
Expand Down Expand Up @@ -1317,7 +1339,7 @@ export async function transactionTest(
baseTransaction.zcount(key8, { value: 2 }, InfBoundary.PositiveInfinity);
responseData.push([
"zcount(key8, { value: 2 }, InfBoundary.PositiveInfinity)",
4,
5,
]);
baseTransaction.zlexcount(
key8,
Expand All @@ -1326,17 +1348,17 @@ export async function transactionTest(
);
responseData.push([
'zlexcount(key8, { value: "a" }, InfBoundary.PositiveInfinity)',
4,
6,
]);
baseTransaction.zpopmin(key8);
responseData.push([
"zpopmin(key8)",
convertRecordToGlideRecord({ member2: 3.0 }),
convertRecordToGlideRecord({ negInfMember: -Infinity }),
]);
baseTransaction.zpopmax(key8);
responseData.push([
"zpopmax(key8)",
convertRecordToGlideRecord({ member5: 5 }),
convertRecordToGlideRecord({ infMember: Infinity }),
]);
baseTransaction.zadd(key8, [{ element: member6, score: 6 }]);
responseData.push(["zadd(key8, {member6: 6})", 1]);
Expand All @@ -1359,7 +1381,7 @@ export async function transactionTest(
InfBoundary.NegativeInfinity,
InfBoundary.PositiveInfinity,
);
responseData.push(["zremRangeByScore(key8, -Inf, +Inf)", 1]); // key8 is now empty
responseData.push(["zremRangeByScore(key8, -Inf, +Inf)", 3]); // key8 is now empty
baseTransaction.zremRangeByLex(
key8,
InfBoundary.NegativeInfinity,
Expand Down
Loading