Skip to content

Fix inclusive bounds handling in Position encoding #70

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

Closed
wants to merge 1 commit into from
Closed

Fix inclusive bounds handling in Position encoding #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
227 changes: 227 additions & 0 deletions src/client/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,4 +497,231 @@ describe("TableAggregate with namespace", () => {
expect(vacationCount).toBe(1);
});
});

describe("inclusive bounds behavior", () => {
let t: ConvexTest;
let aggregate: ReturnType<typeof createAggregates>["aggregate"];

beforeEach(() => {
t = setupTest();
({ aggregate } = createAggregates());
});

test("should respect inclusive vs exclusive lower bounds", async () => {
await t.run(async (ctx) => {
const docs = [
{ name: "item1", value: 10 },
{ name: "item2", value: 20 },
{ name: "item3", value: 30 },
];

for (const doc of docs) {
const id = await ctx.db.insert("testItems", doc);
const insertedDoc = await ctx.db.get(id);
await aggregate.insert(ctx, insertedDoc!);
}
});

const countInclusiveLower = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: true },
},
});
});
expect(countInclusiveLower).toBe(2);

const countExclusiveLower = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: false },
},
});
});
expect(countExclusiveLower).toBe(1);
});

test("should respect inclusive vs exclusive upper bounds", async () => {
await t.run(async (ctx) => {
const docs = [
{ name: "item1", value: 10 },
{ name: "item2", value: 20 },
{ name: "item3", value: 30 },
];

for (const doc of docs) {
const id = await ctx.db.insert("testItems", doc);
const insertedDoc = await ctx.db.get(id);
await aggregate.insert(ctx, insertedDoc!);
}
});

const countInclusiveUpper = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
upper: { key: 20, inclusive: true },
},
});
});
expect(countInclusiveUpper).toBe(2);

const countExclusiveUpper = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
upper: { key: 20, inclusive: false },
},
});
});
expect(countExclusiveUpper).toBe(1);
});

test("should respect inclusive vs exclusive bounds with both lower and upper", async () => {
await t.run(async (ctx) => {
const docs = [
{ name: "item1", value: 10 },
{ name: "item2", value: 20 },
{ name: "item3", value: 30 },
{ name: "item4", value: 40 },
];

for (const doc of docs) {
const id = await ctx.db.insert("testItems", doc);
const insertedDoc = await ctx.db.get(id);
await aggregate.insert(ctx, insertedDoc!);
}
});

const countBothInclusive = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: true },
upper: { key: 30, inclusive: true },
},
});
});
expect(countBothInclusive).toBe(2);

const countBothExclusive = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: false },
upper: { key: 30, inclusive: false },
},
});
});
expect(countBothExclusive).toBe(0);

const countMixed1 = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: true },
upper: { key: 30, inclusive: false },
},
});
});
expect(countMixed1).toBe(1);

const countMixed2 = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: false },
upper: { key: 30, inclusive: true },
},
});
});
expect(countMixed2).toBe(1);
});

test("should respect inclusive bounds with exact boundary matches", async () => {
await t.run(async (ctx) => {
const docs = [
{ name: "item1", value: 15 },
{ name: "item2", value: 20 },
{ name: "item3", value: 20 },
{ name: "item4", value: 25 },
];

for (const doc of docs) {
const id = await ctx.db.insert("testItems", doc);
const insertedDoc = await ctx.db.get(id);
await aggregate.insert(ctx, insertedDoc!);
}
});

const countInclusiveLowerDupe = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: true },
},
});
});
expect(countInclusiveLowerDupe).toBe(3);

const countExclusiveLowerDupe = await t.run(async (ctx) => {
return await aggregate.count(ctx, {
bounds: {
lower: { key: 20, inclusive: false },
},
});
});
expect(countExclusiveLowerDupe).toBe(1);
});

test("should respect inclusive bounds with array keys", async () => {
const aggregateWithArrayKeys = new TableAggregate(components.aggregate, {
sortKey: (doc) => [doc.value, doc.name],
});

await t.run(async (ctx) => {
const docs = [
{ name: "a", value: 10 },
{ name: "b", value: 20 },
{ name: "c", value: 20 },
{ name: "d", value: 30 },
];

for (const doc of docs) {
const id = await ctx.db.insert("testItems", doc);
const insertedDoc = await ctx.db.get(id);
await aggregateWithArrayKeys.insert(ctx, insertedDoc!);
}
});

const countInclusiveArrayLower = await t.run(async (ctx) => {
return await aggregateWithArrayKeys.count(ctx, {
bounds: {
lower: { key: [20, "b"], inclusive: true },
},
});
});
expect(countInclusiveArrayLower).toBe(3);

const countExclusiveArrayLower = await t.run(async (ctx) => {
return await aggregateWithArrayKeys.count(ctx, {
bounds: {
lower: { key: [20, "b"], inclusive: false },
},
});
});
expect(countExclusiveArrayLower).toBe(2);

const countInclusiveArrayUpper = await t.run(async (ctx) => {
return await aggregateWithArrayKeys.count(ctx, {
bounds: {
upper: { key: [20, "c"], inclusive: true },
},
});
});
expect(countInclusiveArrayUpper).toBe(3);

const countExclusiveArrayUpper = await t.run(async (ctx) => {
return await aggregateWithArrayKeys.count(ctx, {
bounds: {
upper: { key: [20, "c"], inclusive: false },
},
});
});
expect(countExclusiveArrayUpper).toBe(2);
});
});
});
15 changes: 6 additions & 9 deletions src/client/positions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ const AFTER_ALL_IDS: never[] = [];

// First a key, which is exploded with explodeKey.
// Then the ID, or BEFORE_ALL_IDS or AFTER_ALL_IDS.
// Then a value to be inclusive or exclusive.
export type Position = [Key, string | null | never[], "" | null | never[]];
export type Position = [Key, string | null | never[]];

function explodeKey<K extends Key>(key: K): Key {
if (Array.isArray(key)) {
Expand Down Expand Up @@ -68,7 +67,7 @@ export function keyToPosition<K extends Key, ID extends string>(
key: K,
id: ID
): Position {
return [explodeKey(key), id, ""];
return [explodeKey(key), id];
}

export function positionToKey<K extends Key, ID extends string>(
Expand All @@ -91,8 +90,8 @@ export function boundsToPositions<K extends Key, ID extends string>(
exploded.push(item);
}
return {
k1: [exploded.concat([BEFORE_ALL_IDS]), BEFORE_ALL_IDS, BEFORE_ALL_IDS],
k2: [exploded.concat([AFTER_ALL_IDS]), AFTER_ALL_IDS, AFTER_ALL_IDS],
k1: [exploded.concat([BEFORE_ALL_IDS]), BEFORE_ALL_IDS],
k2: [exploded.concat([AFTER_ALL_IDS]), AFTER_ALL_IDS],
};
}
return {
Expand Down Expand Up @@ -120,14 +119,12 @@ export function boundToPosition<K extends Key, ID extends string>(
if (direction === "lower") {
return [
explodeKey(bound.key),
bound.id ?? BEFORE_ALL_IDS,
bound.inclusive ? BEFORE_ALL_IDS : AFTER_ALL_IDS,
bound.id ?? (bound.inclusive ? BEFORE_ALL_IDS : AFTER_ALL_IDS),
];
} else {
return [
explodeKey(bound.key),
bound.id ?? AFTER_ALL_IDS,
bound.inclusive ? AFTER_ALL_IDS : BEFORE_ALL_IDS,
bound.id ?? (bound.inclusive ? AFTER_ALL_IDS : BEFORE_ALL_IDS),
];
}
}
Loading