Skip to content

Conversation

@ianmacartney
Copy link
Contributor

Currently if your aggregate key is not an array, prefix doesn't work (gives an opaque type error).
Instead, allow doing eq since this is likely what you want.

And don't allow "prefix" when the key is not an array.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 30, 2025

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/aggregate/@convex-dev/aggregate@159

commit: 69691cd

Copy link
Member

@Nicolapps Nicolapps left a comment

Choose a reason for hiding this comment

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

Thank you! Can you please add a unit test for this?

@ianmacartney
Copy link
Contributor Author

@coderabbitai can you write a few targeted tests here?

@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the list of files changed in this PR
gh pr view 159 --json files --jq '.files[].path'

Length of output: 43


🏁 Script executed:

#!/bin/bash
# Get the diff for this PR to understand the changes
gh pr diff 159

Length of output: 1336


🏁 Script executed:

#!/bin/bash
# Find existing test files to understand the test structure
fd -e test.ts -e spec.ts

Length of output: 138


🏁 Script executed:

#!/bin/bash
# Look for test files related to aggregates
fd -e ts . | rg -i test | rg -i aggregate

Length of output: 32


🏁 Script executed:

#!/bin/bash
# Look at the client test file to understand test structure
cat src/client/index.test.ts

Length of output: 28067


🏁 Script executed:

#!/bin/bash
# Look at the positions.ts file to understand the full context
cat src/client/positions.ts

Length of output: 3872


🏁 Script executed:

#!/bin/bash
# Check if there are any existing tests for boundsToPositions
rg -n "boundsToPositions" src/client/

Length of output: 795


@ianmacartney, I'll write targeted tests for the new {eq: key} bounds functionality. Based on the PR changes, I'll test:

  1. {eq: value} with non-array keys (the main new feature)
  2. {eq: [array]} with array keys
  3. Verification that existing prefix functionality still works

Here are the tests to add to src/client/index.test.ts:

describe("Bounds with eq", () => {
  let t: ConvexTest;
  let aggregate: ReturnType<typeof createAggregates>["aggregate"];

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

  test("should count items with eq bound on non-array key", async () => {
    await t.run(async (ctx) => {
      const docs = [
        { name: "item1", value: 10 },
        { name: "item2", value: 20 },
        { name: "item3", value: 20 },
        { name: "item4", 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!);
      }
    });

    // Test eq bound - should only count items with exact key value
    const countEq20 = await t.run(async (ctx) => {
      return await aggregate.count(ctx, {
        bounds: { eq: 20 },
      });
    });
    expect(countEq20).toBe(2); // Two items with value 20

    const countEq10 = await t.run(async (ctx) => {
      return await aggregate.count(ctx, {
        bounds: { eq: 10 },
      });
    });
    expect(countEq10).toBe(1); // One item with value 10

    const countEq30 = await t.run(async (ctx) => {
      return await aggregate.count(ctx, {
        bounds: { eq: 30 },
      });
    });
    expect(countEq30).toBe(1); // One item with value 30

    const countEq40 = await t.run(async (ctx) => {
      return await aggregate.count(ctx, {
        bounds: { eq: 40 },
      });
    });
    expect(countEq40).toBe(0); // No items with value 40
  });

  test("should sum items with eq bound on non-array key", async () => {
    await t.run(async (ctx) => {
      const docs = [
        { name: "item1", value: 15 },
        { name: "item2", value: 15 },
        { name: "item3", 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 sumEq15 = await t.run(async (ctx) => {
      return await aggregate.sum(ctx, {
        bounds: { eq: 15 },
      });
    });
    expect(sumEq15).toBe(30); // 15 + 15 = 30

    const sumEq25 = await t.run(async (ctx) => {
      return await aggregate.sum(ctx, {
        bounds: { eq: 25 },
      });
    });
    expect(sumEq25).toBe(25);

    const sumEq100 = await t.run(async (ctx) => {
      return await aggregate.sum(ctx, {
        bounds: { eq: 100 },
      });
    });
    expect(sumEq100).toBe(0); // No items with value 100
  });

  test("should work with eq bound on array keys", async () => {
    const aggregateWithArrayKeys = new TableAggregate(components.aggregate, {
      sortKey: (doc) => [doc.value, doc.name],
      sumValue: (doc) => doc.value,
    });

    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!);
      }
    });

    // Test eq bound with exact array match
    const countEqArray = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.count(ctx, {
        bounds: { eq: [20, "b"] },
      });
    });
    expect(countEqArray).toBe(1); // Only one item with exact key [20, "b"]

    const countEqArray2 = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.count(ctx, {
        bounds: { eq: [20, "c"] },
      });
    });
    expect(countEqArray2).toBe(1); // Only one item with exact key [20, "c"]

    const countEqArrayNonExistent = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.count(ctx, {
        bounds: { eq: [20, "z"] },
      });
    });
    expect(countEqArrayNonExistent).toBe(0); // No items with key [20, "z"]
  });

  test("should work with eq bound and namespace", async () => {
    const { aggregateWithNamespace } = createAggregates();

    await t.run(async (ctx) => {
      const docs = [
        { album: "vacation", url: "photo1.jpg", score: 10 },
        { album: "vacation", url: "photo2.jpg", score: 20 },
        { album: "vacation", url: "photo3.jpg", score: 20 },
        { album: "family", url: "photo4.jpg", score: 20 },
      ];

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

    // Test eq bound within vacation namespace
    const vacationCountEq20 = await t.run(async (ctx) => {
      return await aggregateWithNamespace.count(ctx, {
        namespace: "vacation",
        bounds: { eq: 20 },
      });
    });
    expect(vacationCountEq20).toBe(2); // Two vacation photos with score 20

    // Test eq bound within family namespace
    const familyCountEq20 = await t.run(async (ctx) => {
      return await aggregateWithNamespace.count(ctx, {
        namespace: "family",
        bounds: { eq: 20 },
      });
    });
    expect(familyCountEq20).toBe(1); // One family photo with score 20

    // Test eq bound with non-existent value in namespace
    const vacationCountEq100 = await t.run(async (ctx) => {
      return await aggregateWithNamespace.count(ctx, {
        namespace: "vacation",
        bounds: { eq: 100 },
      });
    });
    expect(vacationCountEq100).toBe(0);
  });

  test("should use eq bounds in batch operations", async () => {
    await t.run(async (ctx) => {
      const docs = [
        { name: "item1", value: 10 },
        { name: "item2", value: 20 },
        { name: "item3", value: 20 },
        { name: "item4", 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!);
      }
    });

    // Test countBatch with multiple eq bounds
    const counts = await t.run(async (ctx) => {
      return await aggregate.countBatch(ctx, [
        { bounds: { eq: 10 } },
        { bounds: { eq: 20 } },
        { bounds: { eq: 30 } },
        { bounds: { eq: 40 } },
      ]);
    });
    expect(counts).toEqual([1, 2, 1, 0]);

    // Test sumBatch with eq bounds
    const sums = await t.run(async (ctx) => {
      return await aggregate.sumBatch(ctx, [
        { bounds: { eq: 10 } },
        { bounds: { eq: 20 } },
        { bounds: { eq: 30 } },
      ]);
    });
    expect(sums).toEqual([10, 40, 30]);
  });
});

describe("Bounds with prefix on array keys", () => {
  let t: ConvexTest;

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

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

    await t.run(async (ctx) => {
      const docs = [
        { name: "a", value: 10 },
        { name: "b", value: 10 },
        { name: "c", value: 20 },
        { name: "d", value: 20 },
        { name: "e", 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!);
      }
    });

    // Test prefix bound - should match all items with value 10
    const countPrefix10 = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.count(ctx, {
        bounds: { prefix: [10] },
      });
    });
    expect(countPrefix10).toBe(2); // Two items with first element 10

    const countPrefix20 = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.count(ctx, {
        bounds: { prefix: [20] },
      });
    });
    expect(countPrefix20).toBe(2); // Two items with first element 20

    // Test empty prefix - should match all items
    const countPrefixEmpty = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.count(ctx, {
        bounds: { prefix: [] },
      });
    });
    expect(countPrefixEmpty).toBe(5); // All items

    // Test full prefix - should match exact item
    const countPrefixFull = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.count(ctx, {
        bounds: { prefix: [10, "a"] },
      });
    });
    expect(countPrefixFull).toBe(1); // Only [10, "a"]
  });

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

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

      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 sumPrefix10 = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.sum(ctx, {
        bounds: { prefix: [10] },
      });
    });
    expect(sumPrefix10).toBe(20); // 10 + 10 = 20

    const sumPrefix20 = await t.run(async (ctx) => {
      return await aggregateWithArrayKeys.sum(ctx, {
        bounds: { prefix: [20] },
      });
    });
    expect(sumPrefix20).toBe(20);
  });
});

These tests cover:

  1. eq with non-array keys (the main new feature from the PR) - testing count, sum, and with namespaces
  2. eq with array keys - ensuring it works for exact array matches
  3. eq in batch operations - countBatch and sumBatch
  4. prefix still works with array keys - verifying the existing functionality isn't broken

The tests follow the existing patterns in the test file and should provide good coverage of the new functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants