Abdulramon Jemil•5mo ago

Bad example in docs

In the vector search docs (https://docs.convex.dev/search/vector-search#using-a-separate-table-to-store-vectors), there is the following example:

export const fetchMovies = query({
  args: {
    ids: v.array(v.id("movieEmbeddings")),
  },
  handler: async (ctx, args) => {
    const results = [];
    for (const id of args.ids) {
      const doc = await ctx.db
        .query("movies")
        .withIndex("by_embedding", (q) => q.eq("embeddingId", id))
        .unique();
      if (doc === null) {
        continue;
      }
      results.push(doc);
    }
    return results;
  },
});

export const fetchMovies = query({
  args: {
    ids: v.array(v.id("movieEmbeddings")),
  },
  handler: async (ctx, args) => {
    const results = [];
    for (const id of args.ids) {
      const doc = await ctx.db
        .query("movies")
        .withIndex("by_embedding", (q) => q.eq("embeddingId", id))
        .unique();
      if (doc === null) {
        continue;
      }
      results.push(doc);
    }
    return results;
  },
});

This example uses await inside of a for loop, which I believe is from convex-demos/vector-search (https://github.com/get-convex/convex-demos/blob/d7e772bf8dcf694a075556c639a0c645b833e4a4/vector-search/convex/movies.ts#L110), and this is, in general a bad practice, and it becomes evident when there's a large number of ids to enumerate, resulting in very long response times. A better solution would be to use Promise.all and Array.prototype.filter as follows:

export const fetchMovies = query({
  args: {
    ids: v.array(v.id("movieEmbeddings"))
  },
  handler: async (ctx, args) => {
    const results = await Promise.all(
      args.ids.map((id) =>
        ctx.db
          .query("movies")
          .withIndex("by_embedding", (q) => q.eq("embeddingId", id))
          .unique()
      )
    )
    return results.filter((result) => result !== null)
  }
})

export const fetchMovies = query({
  args: {
    ids: v.array(v.id("movieEmbeddings"))
  },
  handler: async (ctx, args) => {
    const results = await Promise.all(
      args.ids.map((id) =>
        ctx.db
          .query("movies")
          .withIndex("by_embedding", (q) => q.eq("embeddingId", id))
          .unique()
      )
    )
    return results.filter((result) => result !== null)
  }
})

1 Reply

Indy•5mo ago

Ah yep. Promise.all is a bit more efficient since it allows for parallel lookups. I believe these docs were written before we added parallelism 🙂 I'll make a note internally. Thanks for calling it out.

Bad example in docs

Did you find this page helpful?