Skip to content

ORM: any 2+ field orderBy forces a full collect() + JS sort, even when a compound index serves it exactly #416

Closed

Description

@MikeyZhang75

Summary

Any orderBy with two or more fields forces a full collect() plus a JS sort, even when a declared compound index serves the sort exactly. The bail is on arity alone — it never checks direction or index prefix.

Measured on origin/main (200-row posts, index numLikesAndType on (type, numLikes)):

Query limit Reads
orderBy: [asc(type)] 5 5
orderBy: [asc(type), asc(numLikes)] 5 200
orderBy: [asc(type), asc(numLikes)] 50 200

Limit-independent — adding a second sort field on an index that already serves it costs the whole table.

Root cause

There are two independent blanket bails. Fixing either one alone changes nothing.

1. The pushdown helper bails on arity

packages/kitcn/src/orm/index-utils.ts:139:

  if (!indexFields || orderSpecs.length !== 1) {
    return null;
  }

The stated rationale, index-utils.ts:130:

  /**
   * Requested sort. `.order()` reverses the whole key tuple rather than one
   * column, so only a single spec is ever servable.
   */
  orderSpecs: readonly OrderSpec[];

The premise is correct — Convex's .order() flips the entire key tuple — but the conclusion is too strong. A multi-field sort is servable when every unpinned spec shares one direction and the specs are a prefix of the index in order.

2. The planner bails again, downstream

packages/kitcn/src/orm/query.ts:6118 and :6207:

    const hasSecondaryOrders = postFetchOrders.length > 1;
    usePostFetchSort = needsPostFetchSortForPrimary || hasSecondaryOrders;

That flows into query.ts:6884:

    const paginateAfterPostFetchSort =
      usePostFetchSort && postFetchOrders.length > 0;

and finally to the read site at query.ts:6916-6927:

    } else if (
      limit === undefined ||
      paginateAfterPostFetchSort ||
      hasResidualPostFilter ||
      hasPostFetchMembership
    ) {
      // No stream available (no defineSchema()) or no limit to push down.
      // Correctness wins over the read bound: scan, then size below.
      rows = await query.collect();
    } else {
      rows = await query.take(offset > 0 ? offset + limit : limit);
    }

3. The helper never sees the secondary specs anyway

All three planner call sites truncate before calling it — query.ts:6138, :6168, :6396 pass orderSpecs: [primaryOrder]. So narrowing the helper without fixing the call sites is a no-op.

Scope beyond the audit

  • The multi-probe union path has its own separate bail at query.ts:6404.
  • The relation loader shares the bug through the same helper (query.ts:8651-8661, collect at :8726) — measured 201 reads vs 6 achievable.

Constraints a naive patch will hit

  1. Null ordering diverges. _compareByOrderSpecs (query.ts:895) puts null/undefined last in both directions; a Convex index puts missing values first. Verified on origin/main against users.deletedAt / by_deleted_at. Pushdown must be skipped when any sort field is an optional column, or output silently changes.
  2. Tie-breaks change. Array.prototype.sort is stable, so ties currently resolve in index-scan order; a pushed-down sort resolves them by the implicit trailing _creationTime. Usually identical, not guaranteed.
  3. _creationTime is only servable as the final spec, and only once every declared index field is pinned or consumed (index-utils.ts:155-158). Note orderBy: { createdAt } normalizes to _creationTime.
  4. Index selection is leading-field-only at query.ts:6185-6192 (idx.fields[0] === orderField). The no-where branch needs selection over the whole spec list.
  5. Two branches cannot switch index at all — the configured-index branch (:6147) and the where-compiled branch (:6122) have the index already pinned. There the fix can only answer yes/no.
  6. eq-pinned fields are already handled (index-utils.ts:147-151) and may match in any position, any direction — a multi-spec version must preserve that.

Suggested fix

Replace the arity bail with a real servability check inside resolveIndexOrderPushdown: consume specs pinned by the eq prefix in any order/direction, require the remainder to match indexFields[eqCount], indexFields[eqCount+1], … in order, allow _creationTime only as the final spec, and require all unpinned specs to share one direction. Return that direction, else null.

Then pass the full postFetchOrders at :6138, :6168, :6396, and change :6207 to consult the pushdown result rather than arity. Narrow :6404 the same way. Keep _compareByOrderSpecs as the fallback for mixed directions and non-prefix sorts.

Existing tests

packages/kitcn/src/orm/index-utils.test.ts:300-311 — "declines multi-field sorts because .order() reverses the whole key". Its fixture is genuinely unservable (mixed directions and a non-prefix field), so a correctly narrowed helper keeps it green; only the test name goes stale.

A regression test should assert the same absolute bound at two table sizes, since a scan-shaped plan cannot satisfy both.

Environment

  • kitcn 0.25.14
  • convex 1.44.0
  • verified and measured against origin/main at 3bbc6bb3

Activity

  1. self-assigned this
    on Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions