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.

Sign up for GitHub

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

fix(*): Method.Random[Key] behavior #269

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

fix(*): Method.Random[Key] behavior #269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c165745
fix(JSONProvider): `Method.Random[Key]` behaviour
RealShadowNova Jun 30, 2023
40162cc
fix(MapProvider): `Method.Random[Key]` behaviour
RealShadowNova Jun 30, 2023
617c62c
fix(MariaProvider): `Method.Random[Key]` behaviour
RealShadowNova Jun 30, 2023
b5fa4a7
fix(PostgreSQLProvider): `Method.Random[Key]` behaviour
RealShadowNova Jun 30, 2023
e57b55f
fix(SQLiteProvider): `Method.Random[Key]` behaviour
RealShadowNova Jun 30, 2023
cd06794
fix: duplicates > unique
WilsontheWolf Jul 1, 2023
f489b59
chore: bump @joshdb/providers
WilsontheWolf Jul 1, 2023
68f2e4b
fix(mongo): random sample when unique is false
dan-online Jul 1, 2023
dd7f4e0
chore: remove todo
WilsontheWolf Jul 9, 2023
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
12 changes: 6 additions & 6 deletions benchmarks/src/lib/constants/benchmark-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ export const BENCHMARK_TESTS: Benchmark.Test[] = [
}
},
{
name: `${Method.Random} (Duplicates)`,
name: `${Method.Random} (Unique)`,

beforeAll: async ({ provider, entries }) => {
await provider[Method.SetMany]({
Expand All @@ -402,7 +402,7 @@ export const BENCHMARK_TESTS: Benchmark.Test[] = [
},

run: async ({ provider }) => {
await provider[Method.Random]({ method: Method.Random, errors: [], count: 5, duplicates: true });
await provider[Method.Random]({ method: Method.Random, errors: [], count: 5, unique: true });
}
},
{
Expand All @@ -418,11 +418,11 @@ export const BENCHMARK_TESTS: Benchmark.Test[] = [
},

run: async ({ provider }) => {
await provider[Method.Random]({ method: Method.Random, errors: [], count: 5, duplicates: false });
await provider[Method.Random]({ method: Method.Random, errors: [], count: 5, unique: false });
}
},
{
name: `${Method.RandomKey} (Duplicates)`,
name: `${Method.RandomKey} (Unique)`,

beforeAll: async ({ provider, entries }) => {
await provider[Method.SetMany]({
Expand All @@ -434,7 +434,7 @@ export const BENCHMARK_TESTS: Benchmark.Test[] = [
},

run: async ({ provider }) => {
await provider[Method.RandomKey]({ method: Method.RandomKey, errors: [], count: 5, duplicates: true });
await provider[Method.RandomKey]({ method: Method.RandomKey, errors: [], count: 5, unique: true });
}
},
{
Expand All @@ -450,7 +450,7 @@ export const BENCHMARK_TESTS: Benchmark.Test[] = [
},

run: async ({ provider }) => {
await provider[Method.RandomKey]({ method: Method.RandomKey, errors: [], count: 5, duplicates: false });
await provider[Method.RandomKey]({ method: Method.RandomKey, errors: [], count: 5, unique: false });
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion packages/json/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"check-update": "cliff-jumper --dry-run"
},
"dependencies": {
"@joshdb/provider": "2.0.0-next.a699598.0",
"@joshdb/provider": "2.0.0-next.095d141.0",
"@sapphire/async-queue": "^1.5.0",
"@sapphire/snowflake": "^3.5.1",
"@sapphire/utilities": "^3.12.0",
Expand Down
42 changes: 26 additions & 16 deletions packages/json/src/lib/JSONProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,60 +550,70 @@ export class JSONProvider<StoredValue = unknown> extends JoshProvider<StoredValu
}

public async [Method.Random](payload: Payload.Random<StoredValue>): Promise<Payload.Random<StoredValue>> {
const { count, duplicates } = payload;
const { count, unique } = payload;
const size = await this.handler.size();

if (size === 0) return { ...payload, data: [] };
if (size < count) {
if (unique && size < count) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.InvalidCount, method: Method.Random }, { size }));

return payload;
}

if (size === 0) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.MissingData, method: Method.Random }, { unique, count }));

return payload;
}

payload.data = [];

const keys = await this.handler.keys();

if (duplicates) {
while (payload.data.length < count) {
const key = keys[Math.floor(Math.random() * size)];

payload.data.push((await this.handler.get(key))!);
}
} else {
if (unique) {
const randomKeys = new Set<string>();

while (randomKeys.size < count) randomKeys.add(keys[Math.floor(Math.random() * keys.length)]);
dan-online marked this conversation as resolved.
Show resolved Hide resolved

for (const key of randomKeys) payload.data.push((await this.handler.get(key))!);
} else {
while (payload.data.length < count) {
const key = keys[Math.floor(Math.random() * size)];

payload.data.push((await this.handler.get(key))!);
}
}

return payload;
}

public async [Method.RandomKey](payload: Payload.RandomKey): Promise<Payload.RandomKey> {
const { count, duplicates } = payload;
const { count, unique } = payload;
const size = await this.handler.size();

if (size === 0) return { ...payload, data: [] };
if (size < count) {
if (unique && size < count) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.InvalidCount, method: Method.RandomKey }, { size }));

return payload;
}

if (size === 0) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.MissingData, method: Method.RandomKey }, { size }));

return payload;
}

payload.data = [];

const keys = await this.handler.keys();

if (duplicates) {
while (payload.data.length < count) payload.data.push(keys[Math.floor(Math.random() * size)]);
} else {
if (unique) {
const randomKeys = new Set<string>();

while (randomKeys.size < count) randomKeys.add(keys[Math.floor(Math.random() * keys.length)]);

for (const key of randomKeys) payload.data.push(key);
} else {
while (payload.data.length < count) payload.data.push(keys[Math.floor(Math.random() * size)]);
}

return payload;
Expand Down
2 changes: 1 addition & 1 deletion packages/map/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"check-update": "cliff-jumper --dry-run"
},
"dependencies": {
"@joshdb/provider": "2.0.0-next.a699598.0",
"@joshdb/provider": "2.0.0-next.095d141.0",
"@sapphire/utilities": "^3.12.0",
"better-serialize": "^1.0.0",
"property-helpers": "^2.0.0"
Expand Down
44 changes: 26 additions & 18 deletions packages/map/src/lib/MapProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,60 +488,68 @@ export class MapProvider<StoredValue = unknown> extends JoshProvider<StoredValue
}

public [Method.Random](payload: Payload.Random<StoredValue>): Payload.Random<StoredValue> {
if (this.cache.size === 0) return { ...payload, data: [] };
const { count, unique } = payload;

const { count, duplicates } = payload;

if (this.cache.size < count) {
if (unique && this.cache.size < count) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.InvalidCount, method: Method.Random }));

return payload;
}

if (this.cache.size === 0) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.MissingData, method: Method.Random, context: { unique, count } }));

return payload;
}

payload.data = [];

const keys = Array.from(this.cache.keys());

if (duplicates) {
while (payload.data.length < count) {
const key = keys[Math.floor(Math.random() * keys.length)];

payload.data.push(this.cache.get(key)!);
}
} else {
if (unique) {
const randomKeys = new Set<string>();

while (randomKeys.size < count) randomKeys.add(keys[Math.floor(Math.random() * keys.length)]);

for (const key of randomKeys) payload.data.push(this.cache.get(key)!);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to not need 2 loops here?

Copy link
Member

Choose a reason for hiding this comment

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

You could probably just payload.data = randomKeys

Copy link
Member Author

Choose a reason for hiding this comment

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

We have to get the value with the keys, so yes.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah

Copy link
Member

Choose a reason for hiding this comment

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

I guess we could probably get the key and fetch the value in the same loop though

} else {
while (payload.data.length < count) {
const key = keys[Math.floor(Math.random() * keys.length)];

payload.data.push(this.cache.get(key)!);
}
}

return payload;
}

public [Method.RandomKey](payload: Payload.RandomKey): Payload.RandomKey {
if (this.cache.size === 0) return { ...payload, data: [] };
const { count, unique } = payload;

const { count, duplicates } = payload;

if (this.cache.size < count) {
if (unique && this.cache.size < count) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.InvalidCount, method: Method.RandomKey }));

return payload;
}

if (this.cache.size === 0) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.MissingData, method: Method.RandomKey }));

return payload;
}

payload.data = [];

const keys = Array.from(this.cache.keys());

if (duplicates) {
while (payload.data.length < count) payload.data.push(keys[Math.floor(Math.random() * keys.length)]);
} else {
if (unique) {
const randomKeys = new Set<string>();

while (randomKeys.size < count) randomKeys.add(keys[Math.floor(Math.random() * keys.length)]);

for (const key of randomKeys) payload.data.push(key);
} else {
while (payload.data.length < count) payload.data.push(keys[Math.floor(Math.random() * keys.length)]);
}

return payload;
Expand Down
2 changes: 1 addition & 1 deletion packages/maria/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"check-update": "cliff-jumper --dry-run"
},
"dependencies": {
"@joshdb/provider": "2.0.0-next.a699598.0",
"@joshdb/provider": "2.0.0-next.095d141.0",
"@sapphire/snowflake": "^3.5.1",
"better-serialize": "^1.0.0",
"mariadb": "^3.2.0"
Expand Down
42 changes: 26 additions & 16 deletions packages/maria/src/lib/MariaProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,60 +517,70 @@ export class MariaProvider<StoredValue = unknown> extends JoshProvider<StoredVal
}

public async [Method.Random](payload: Payload.Random<StoredValue>): Promise<Payload.Random<StoredValue>> {
const { count, duplicates } = payload;
const { count, unique } = payload;
const size = await this.handler.size();

if (size === 0) return { ...payload, data: [] };
if (size < count) {
if (unique && size < count) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.InvalidCount, method: Method.Random }));

return payload;
}

if (size === 0) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.MissingData, method: Method.Random, context: { unique, count } }));

return payload;
}

payload.data = [];

const keys = await this.handler.keys();

if (duplicates) {
while (payload.data.length < count) {
const key = keys[Math.floor(Math.random() * size)];

payload.data.push((await this.handler.get(key))!);
}
} else {
if (unique) {
const randomKeys = new Set<string>();
Copy link
Member

Choose a reason for hiding this comment

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

Does mariadb have any way to query random that would be faster than this?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it does have some kind of random query but its somehow worse than ours
https://mariadb.com/kb/en/rand/

Note, however, that this technique should never be used on a large table as it will be extremely slow. MariaDB will read all rows in the table, generate a random value for each of them, order them, and finally will apply the LIMIT clause.

Copy link
Member

Choose a reason for hiding this comment

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

Can we test this, it's possible while not the best it's better than the current solution

Copy link
Member

Choose a reason for hiding this comment

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

I mean you can. My Maria doesn't work.
However just based off that description, it sounds to me like that is slower. It has to fetch all the keys, generate a random value for each key in the table, sort them and then choose the top x
Whereas ours is fetch all the keys, and repeat the amount of times we need to get, picking a random index


while (randomKeys.size < count) randomKeys.add(keys[Math.floor(Math.random() * keys.length)]);

for (const key of randomKeys) payload.data.push((await this.handler.get(key))!);
} else {
while (payload.data.length < count) {
const key = keys[Math.floor(Math.random() * size)];

payload.data.push((await this.handler.get(key))!);
}
}

return payload;
}

public async [Method.RandomKey](payload: Payload.RandomKey): Promise<Payload.RandomKey> {
const { count, duplicates } = payload;
const { count, unique } = payload;
const size = await this.handler.size();

if (size === 0) return { ...payload, data: [] };
if (size < count) {
if (unique && size < count) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.InvalidCount, method: Method.RandomKey }));

return payload;
}

if (size === 0) {
payload.errors.push(this.error({ identifier: CommonIdentifiers.MissingData, method: Method.RandomKey }));

return payload;
}

payload.data = [];

const keys = await this.handler.keys();

if (duplicates) {
while (payload.data.length < count) payload.data.push(keys[Math.floor(Math.random() * size)]);
} else {
if (unique) {
const randomKeys = new Set<string>();

while (randomKeys.size < count) randomKeys.add(keys[Math.floor(Math.random() * keys.length)]);

for (const key of randomKeys) payload.data.push(key);
} else {
while (payload.data.length < count) payload.data.push(keys[Math.floor(Math.random() * size)]);
}

return payload;
Expand Down
2 changes: 1 addition & 1 deletion packages/mongo/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"check-update": "cliff-jumper --dry-run"
},
"dependencies": {
"@joshdb/provider": "2.0.0-next.a699598.0",
"@joshdb/provider": "2.0.0-next.095d141.0",
"@sapphire/utilities": "^3.12.0",
"better-serialize": "^1.0.0",
"mongodb": "~5.6.0",
Expand Down
Loading