arcadegame.cloud

Prompt

This PR builds the payload for our /api/orders endpoint — the orders list a customer sees, and one of the highest-traffic reads we serve. It passes review on correctness and the output looks right in staging. Review it and tell me whether you'd approve.

async function getOrdersWithCustomers(db) {
  const orders = await db.query("SELECT * FROM orders WHERE status = 'open'");

  const result = [];
  for (const order of orders) {
    const customer = await db.query(
      "SELECT * FROM customers WHERE id = $1",
      [order.customer_id]
    );
    result.push({ ...order, customer: customer[0] });
  }

  return result;
}

How this round runs

Rank what you find by severity and weigh the hot-path framing. The output is correct — say why that doesn't make it approvable, and give the fix.

Model answer

Block. The output is right; the access pattern is the problem.

1 (highest — N+1 query pattern). The function runs one query for the orders, then one more query per order inside the loop to fetch that order's customer. For N open orders that's 1 + N round-trips to the database. The result is correct, which is exactly why a correctness-only review waved it through — the defect is in how it scales, not in what it returns. On the highest-traffic read we serve, every extra order multiplies DB round-trips and connection-pool pressure; under load this is a latency cliff and a pool-exhaustion risk, not a micro-optimization. That's why "it's correct" doesn't earn an approve.

Root cause: the join between orders and customers is being done in application code, one row at a time, instead of in the database. Two correct fixes:

Batch the customer lookups into a single IN query (keeps the two-step shape if you need it):

async function getOrdersWithCustomers(db) {
  const orders = await db.query("SELECT * FROM orders WHERE status = 'open'");
  const ids = [...new Set(orders.map((o) => o.customer_id))];

  const customers = await db.query(
    "SELECT * FROM customers WHERE id = ANY($1)",
    [ids]
  );
  const byId = new Map(customers.map((c) => [c.id, c]));

  return orders.map((o) => ({ ...o, customer: byId.get(o.customer_id) }));
}

Or, better when you always need both, push the join to the database so it's one round-trip total:

SELECT o.*, c.id AS customer_id, c.name AS customer_name
FROM orders o
JOIN customers c ON c.id = o.customer_id
WHERE o.status = 'open';

The IN/ANY version is 1 + 1 queries (de-duped ids matter when many orders share a customer); the join is exactly one. Either turns O(N) round-trips into O(1). If the ORM is generating this, the equivalent is an eager-load / include so it batches instead of lazy-loading per row.

I'd block until it's batched, and I'd ask for the open-order count in production so we know whether N is 50 or 50,000 — but the pattern is wrong regardless of the current number. Style and naming are fine; the block is purely the query pattern.

Signals — what a strong answer shows

Named it as N+1 (1 + N round-trips) rather than a vague 'this is slow'
Explained why correct output still fails review — the defect is in scaling, not the result
Gave two real fixes (batched IN/ANY and a DB-side join) and said when each is preferable
De-duped customer ids in the batch version — caught that many orders share a customer
Used the hot-path framing to justify a block and asked for the production N to size the impact

Follow-ups — where it goes next

When is the loop-of-queries actually acceptable? → When N is tiny and bounded, or the per-row calls hit different stores you can't join — rare on a hot path.
If this is an ORM, where does N+1 come from? → Lazy-loading a relation inside a loop; fix with eager-load/include/join-fetch so it batches.
Join vs IN-query — how do you choose? → Join when you always need both and they're in one DB; IN when you want to keep the steps separate or reuse the customer fetch.

Study this: Databases

N+1 Query in a Hot Path

Prompt

How this round runs

Model answer