fix: add compound [token, expiresAt] index on session table#314
fix: add compound [token, expiresAt] index on session table#314elpiarthera wants to merge 2 commits into
Conversation
When using the multi-session plugin, queries on the session table filter by token eq + expiresAt range. Without a compound index, this falls back to a full table scan, triggering a "Querying without an index" warning. Fixes get-convex#305
|
Someone is attempting to deploy a commit to the Convex Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a composite index ["token","expiresAt"] to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/create-schema.ts (1)
128-135:⚠️ Potential issue | 🔴 CriticalThe
.sort()breaks compound index field ordering.The alphabetical sort ensures consistent naming but breaks Convex's field-order requirement. For
["token", "expiresAt"], this produces["expiresAt", "token"]- the opposite of what the query needs.Consider preserving the original field order and deriving the index name from it:
🛠️ Proposed fix to preserve field order
const indexes = mergedIndexFields(tables)[ tableKey as keyof typeof mergedIndexFields ]?.map((index) => { - const indexArray = Array.isArray(index) ? index.sort() : [index]; - const indexName = indexArray.join("_"); + const indexArray = Array.isArray(index) ? index : [index]; + const indexName = [...indexArray].sort().join("_"); return `.index("${indexName}", ${JSON.stringify(indexArray)})`; }) || [];This preserves the field order for the index definition while still sorting only for the name (for consistency). Alternatively, keep field order in the name as well since it's more descriptive of the query pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/create-schema.ts` around lines 128 - 135, The current logic in create-schema.ts uses Array.prototype.sort() on index arrays (see mergedIndexFields, tableKey) which reorders fields and breaks Convex's required field ordering; change the code so you do NOT sort the actual index array used for the index definition (preserve original order for index creation) but still derive a stable index name by creating a separate sorted copy just for naming (e.g., keep the original indexArray as-is for the .index(...) argument and compute indexName from a sorted shallow copy), ensuring indexName remains deterministic while the index field order stays intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/client/create-schema.ts`:
- Around line 128-135: The current logic in create-schema.ts uses
Array.prototype.sort() on index arrays (see mergedIndexFields, tableKey) which
reorders fields and breaks Convex's required field ordering; change the code so
you do NOT sort the actual index array used for the index definition (preserve
original order for index creation) but still derive a stable index name by
creating a separate sorted copy just for naming (e.g., keep the original
indexArray as-is for the .index(...) argument and compute indexName from a
sorted shallow copy), ensuring indexName remains deterministic while the index
field order stays intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a477309-44cd-411a-b535-e5bd0cafbcd1
📒 Files selected for processing (2)
src/client/create-schema.tssrc/component/schema.ts
The .sort() on index arrays was reordering compound index fields alphabetically, breaking order-dependent queries. Sort only the copy used for the index name, keep original order for the definition.
|
Thanks for this - coderabbit's callout is legit and points to a bigger issue (and coderabbit's fix doesn't actually fix it). Not brought on by your pr but we've been getting away with it because all of our compound indexes so far happened to be in alphabetical order and the one you're adding isn't. Will look at this soon. |
Summary
["token", "expiresAt"]index to session table in both the schema generator (create-schema.ts) and the auto-generated component schematokeneq +expiresAtrangeTest plan
npm run buildpassesnpm run lintpasses (0 errors)npm testpasses (34/34)token_expiresAt)Fixes #305
Orchestrator: Zeta — VantageOS Team Dev | 2026-04-06
Summary by CodeRabbit