add createdAt field to index orders table#3327
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request adds a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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)
indexer/services/ender/src/scripts/handlers/dydx_stateful_order_handler.sql (1)
100-142:⚠️ Potential issue | 🟡 MinorKeep
createdAtin sync in theON CONFLICTupdate branch.
ON CONFLICTcurrently updatescreatedAtHeightbut notcreatedAt, which can leave inconsistent creation metadata when a row conflicts.🛠️ Suggested patch
) ON CONFLICT ("id") DO UPDATE SET "subaccountId" = order_record."subaccountId", "clientId" = order_record."clientId", "clobPairId" = order_record."clobPairId", "side" = order_record."side", "size" = order_record."size", "totalFilled" = order_record."totalFilled", "price" = order_record."price", "timeInForce" = order_record."timeInForce", "reduceOnly" = order_record."reduceOnly", "orderFlags" = order_record."orderFlags", "goodTilBlockTime" = order_record."goodTilBlockTime", "clientMetadata" = order_record."clientMetadata", "createdAtHeight" = order_record."createdAtHeight", + "createdAt" = order_record."createdAt", "updatedAt" = order_record."updatedAt", "updatedAtHeight" = order_record."updatedAtHeight", "type" = order_record."type", "status" = order_record."status", "triggerPrice" = order_record."triggerPrice", "builderAddress" = order_record."builderAddress", "feePpm" = order_record."feePpm", "duration" = order_record."duration", "interval" = order_record."interval", "priceTolerance" = order_record."priceTolerance", "orderRouterAddress" = order_record."orderRouterAddress"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@indexer/services/ender/src/scripts/handlers/dydx_stateful_order_handler.sql` around lines 100 - 142, The ON CONFLICT UPDATE is missing assignment for "createdAt", causing creation timestamp drift; update the conflict branch in this statement so the UPDATE SET includes "createdAt" = order_record."createdAt" (alongside "createdAtHeight" = order_record."createdAtHeight") to keep createdAt in sync for the orders record (refer to the INSERT INTO orders ... VALUES (...) ... ON CONFLICT ... UPDATE SET block and the order_record fields).
🧹 Nitpick comments (1)
indexer/packages/postgres/src/db/migrations/migration_files/20260302120000_add_created_at_to_orders.ts (1)
3-7: Consider a one-time backfill plan for historical rows.This migration adds
createdAtas nullable but leaves existingordersrows null. If consumers are expected to rely on this field broadly, a follow-up backfill (or explicit contract note) would reduce response heterogeneity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@indexer/packages/postgres/src/db/migrations/migration_files/20260302120000_add_created_at_to_orders.ts` around lines 3 - 7, The migration adds a nullable createdAt column on the orders table which leaves historical rows null; create a follow-up one-time backfill migration (or extend this migration) that sets orders.createdAt for existing rows using the best available source (for example an existing timestamp column like created_at or inserted_at, or fallback to a deterministic value such as the row's id-derived time or NOW() if nothing else exists), and ensure you update or document the contract for consumers; reference the current migration function up and the orders table / createdAt column when implementing the backfill so reviewers can locate and verify the change.
🤖 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 `@indexer/services/ender/src/scripts/handlers/dydx_stateful_order_handler.sql`:
- Around line 100-142: The ON CONFLICT UPDATE is missing assignment for
"createdAt", causing creation timestamp drift; update the conflict branch in
this statement so the UPDATE SET includes "createdAt" = order_record."createdAt"
(alongside "createdAtHeight" = order_record."createdAtHeight") to keep createdAt
in sync for the orders record (refer to the INSERT INTO orders ... VALUES (...)
... ON CONFLICT ... UPDATE SET block and the order_record fields).
---
Nitpick comments:
In
`@indexer/packages/postgres/src/db/migrations/migration_files/20260302120000_add_created_at_to_orders.ts`:
- Around line 3-7: The migration adds a nullable createdAt column on the orders
table which leaves historical rows null; create a follow-up one-time backfill
migration (or extend this migration) that sets orders.createdAt for existing
rows using the best available source (for example an existing timestamp column
like created_at or inserted_at, or fallback to a deterministic value such as the
row's id-derived time or NOW() if nothing else exists), and ensure you update or
document the contract for consumers; reference the current migration function up
and the orders table / createdAt column when implementing the backfill so
reviewers can locate and verify the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
indexer/packages/postgres/src/db/migrations/migration_files/20260302120000_add_created_at_to_orders.tsindexer/packages/postgres/src/models/order-model.tsindexer/packages/postgres/src/types/db-model-types.tsindexer/packages/postgres/src/types/order-types.tsindexer/services/comlink/public/swagger.jsonindexer/services/comlink/src/request-helpers/request-transformer.tsindexer/services/ender/src/scripts/handlers/dydx_stateful_order_handler.sqlindexer/services/ender/src/scripts/helpers/dydx_liquidation_fill_handler_per_order.sqlindexer/services/ender/src/scripts/helpers/dydx_order_fill_handler_per_order.sql
4ef66d6 to
6ccfda0
Compare
|
@Mergifyio backport release/indexer/v9.x |
✅ Backports have been createdDetails
|
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
createdAttimestamp field in API responses, displaying when each order was created.Tests
createdAtfield.