fix(ledger): add internal foreign keys#4652
Open
GAlexIHU wants to merge 1 commit into
Open
Conversation
GAlexIHU
added
the
release-note/ignore
Contributor
📝 WalkthroughWalkthroughThis PR adds ent schema edges linking LedgerBreakageRecord to LedgerAccount, LedgerSubAccount, LedgerEntry, LedgerTransaction, LedgerTransactionGroup, and itself (with Restrict delete rules), adds a LedgerCustomerAccount → LedgerAccount edge with a typed account_id column, includes matching up/down SQL migrations, and updates a breakage adapter test helper accordingly. ChangesLedger internal FK wiring
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Alright, this one's a nice, tidy schema-and-migration bundle — new edges tying breakage records into the rest of the ledger graph, a fresh typed column, and a test helper that now does the heavy lifting itself. Should be a smooth read through! 🧵 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR adds ledger-internal foreign keys for ledger account and breakage records. The main changes are:
Confidence Score: 4/5The migration path needs a data-shape fix before merging.
tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql Important Files Changed
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql:6
**Column Narrowing Blocks Migration**
When an existing `ledger_customer_accounts` row has an `account_id` longer than 26 characters, this `ALTER COLUMN` fails before the `NOT VALID` foreign key can defer historical cleanup. The original column was unconstrained `character varying`, so a deploy with legacy or manually imported data can stop at this migration even if the FK orphan preflight is clean.
### Issue 2 of 2
tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql:6
**Legacy Orphans Fail Updates**
`NOT VALID` lets old `ledger_customer_accounts.account_id` values skip the initial FK scan, but PostgreSQL still checks the constraint on later updates to those rows. If an existing mapping points at no `ledger_accounts` row, a routine update such as soft-delete or metadata maintenance can start failing with a foreign-key violation after this migration.
Reviews (1): Last reviewed commit: "fix(ledger): add internal foreign keys" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ALTER TABLE "ledger_breakage_records" ADD CONSTRAINT "ledger_breakage_records_ledger_breakage_records_planned_release" FOREIGN KEY ("plan_id") REFERENCES "ledger_breakage_records" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_breakage_records_release_reopens" FOREIGN KEY ("release_id") REFERENCES "ledger_breakage_records" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_entries_source_breakage_records" FOREIGN KEY ("source_entry_id") REFERENCES "ledger_entries" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_sub_accounts_breakage_records" FOREIGN KEY ("breakage_sub_account_id") REFERENCES "ledger_sub_accounts" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_sub_accounts_fbo_breakage_record" FOREIGN KEY ("fbo_sub_account_id") REFERENCES "ledger_sub_accounts" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_breakage_reco" FOREIGN KEY ("breakage_transaction_group_id") REFERENCES "ledger_transaction_groups" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_source_breaka" FOREIGN KEY ("source_transaction_group_id") REFERENCES "ledger_transaction_groups" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transactions_breakage_records" FOREIGN KEY ("breakage_transaction_id") REFERENCES "ledger_transactions" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transactions_source_breakage_rec" FOREIGN KEY ("source_transaction_id") REFERENCES "ledger_transactions" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID; | ||
| -- modify "ledger_customer_accounts" table | ||
| -- Manual post-generation step: keep this foreign key NOT VALID for the same rollout reason as above. | ||
| ALTER TABLE "ledger_customer_accounts" ALTER COLUMN "account_id" TYPE character(26), ADD CONSTRAINT "ledger_customer_accounts_ledger_accounts_customer_accounts" FOREIGN KEY ("account_id") REFERENCES "ledger_accounts" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID; |
Contributor
There was a problem hiding this comment.
When an existing ledger_customer_accounts row has an account_id longer than 26 characters, this ALTER COLUMN fails before the NOT VALID foreign key can defer historical cleanup. The original column was unconstrained character varying, so a deploy with legacy or manually imported data can stop at this migration even if the FK orphan preflight is clean.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql
Line: 6
Comment:
**Column Narrowing Blocks Migration**
When an existing `ledger_customer_accounts` row has an `account_id` longer than 26 characters, this `ALTER COLUMN` fails before the `NOT VALID` foreign key can defer historical cleanup. The original column was unconstrained `character varying`, so a deploy with legacy or manually imported data can stop at this migration even if the FK orphan preflight is clean.
How can I resolve this? If you propose a fix, please make it concise.
| ALTER TABLE "ledger_breakage_records" ADD CONSTRAINT "ledger_breakage_records_ledger_breakage_records_planned_release" FOREIGN KEY ("plan_id") REFERENCES "ledger_breakage_records" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_breakage_records_release_reopens" FOREIGN KEY ("release_id") REFERENCES "ledger_breakage_records" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_entries_source_breakage_records" FOREIGN KEY ("source_entry_id") REFERENCES "ledger_entries" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_sub_accounts_breakage_records" FOREIGN KEY ("breakage_sub_account_id") REFERENCES "ledger_sub_accounts" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_sub_accounts_fbo_breakage_record" FOREIGN KEY ("fbo_sub_account_id") REFERENCES "ledger_sub_accounts" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_breakage_reco" FOREIGN KEY ("breakage_transaction_group_id") REFERENCES "ledger_transaction_groups" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_source_breaka" FOREIGN KEY ("source_transaction_group_id") REFERENCES "ledger_transaction_groups" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transactions_breakage_records" FOREIGN KEY ("breakage_transaction_id") REFERENCES "ledger_transactions" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID, ADD CONSTRAINT "ledger_breakage_records_ledger_transactions_source_breakage_rec" FOREIGN KEY ("source_transaction_id") REFERENCES "ledger_transactions" ("id") ON UPDATE NO ACTION ON DELETE RESTRICT NOT VALID; | ||
| -- modify "ledger_customer_accounts" table | ||
| -- Manual post-generation step: keep this foreign key NOT VALID for the same rollout reason as above. | ||
| ALTER TABLE "ledger_customer_accounts" ALTER COLUMN "account_id" TYPE character(26), ADD CONSTRAINT "ledger_customer_accounts_ledger_accounts_customer_accounts" FOREIGN KEY ("account_id") REFERENCES "ledger_accounts" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID; |
Contributor
There was a problem hiding this comment.
NOT VALID lets old ledger_customer_accounts.account_id values skip the initial FK scan, but PostgreSQL still checks the constraint on later updates to those rows. If an existing mapping points at no ledger_accounts row, a routine update such as soft-delete or metadata maintenance can start failing with a foreign-key violation after this migration.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql
Line: 6
Comment:
**Legacy Orphans Fail Updates**
`NOT VALID` lets old `ledger_customer_accounts.account_id` values skip the initial FK scan, but PostgreSQL still checks the constraint on later updates to those rows. If an existing mapping points at no `ledger_accounts` row, a routine update such as soft-delete or metadata maintenance can start failing with a foreign-key violation after this migration.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openmeter/ent/schema/ledger_breakage_record.go`:
- Around line 130-187: The LedgerBreakageRecord schema is still missing indexes
on several foreign key fields, causing parent-table deletes to scan this table.
Update the Edges definition in LedgerBreakageRecord to ensure the remaining
FK-backed edges, especially source_transaction, breakage_transaction,
fbo_sub_account, breakage_sub_account, and release, are indexed in the generated
schema by adding the appropriate index annotations or schema indexes tied to
those field names.
In `@tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql`:
- Around line 4-6: The combined ALTER TABLE on ledger_customer_accounts is still
doing a blocking column type rewrite, so separate the account_id type change
from the foreign key addition. Update the migration around the
ledger_customer_accounts ALTER TABLE statement so the TYPE character(26) change
is its own step, then add the
ledger_customer_accounts_ledger_accounts_customer_accounts foreign key in a
separate NOT VALID statement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54c3c901-c48c-4c59-bb21-c21a8e2da563
⛔ Files ignored due to path filters (46)
openmeter/ent/db/client.gois excluded by!**/ent/db/**openmeter/ent/db/ledgeraccount.gois excluded by!**/ent/db/**openmeter/ent/db/ledgeraccount/ledgeraccount.gois excluded by!**/ent/db/**openmeter/ent/db/ledgeraccount/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgeraccount_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgeraccount_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgeraccount_update.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerbreakagerecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerbreakagerecord/ledgerbreakagerecord.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerbreakagerecord/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerbreakagerecord_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerbreakagerecord_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerbreakagerecord_update.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercustomeraccount.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercustomeraccount/ledgercustomeraccount.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercustomeraccount/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercustomeraccount_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercustomeraccount_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgercustomeraccount_update.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerentry.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerentry/ledgerentry.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerentry/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerentry_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerentry_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgerentry_update.gois excluded by!**/ent/db/**openmeter/ent/db/ledgersubaccount.gois excluded by!**/ent/db/**openmeter/ent/db/ledgersubaccount/ledgersubaccount.gois excluded by!**/ent/db/**openmeter/ent/db/ledgersubaccount/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgersubaccount_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgersubaccount_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgersubaccount_update.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransaction.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransaction/ledgertransaction.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransaction/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransaction_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransaction_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransaction_update.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransactiongroup.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransactiongroup/ledgertransactiongroup.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransactiongroup/where.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransactiongroup_create.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransactiongroup_query.gois excluded by!**/ent/db/**openmeter/ent/db/ledgertransactiongroup_update.gois excluded by!**/ent/db/**openmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**openmeter/ent/db/mutation.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (9)
openmeter/ent/schema/ledger_account.goopenmeter/ent/schema/ledger_breakage_record.goopenmeter/ent/schema/ledger_customer_account.goopenmeter/ent/schema/ledger_entry.goopenmeter/ent/schema/ledger_transaction.goopenmeter/ent/schema/ledger_transaction_group.goopenmeter/ledger/breakage/adapter/record_test.gotools/migrate/migrations/20260706130134_add_ledger_internal_fks.down.sqltools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql
Comment on lines
+130
to
+187
| func (LedgerBreakageRecord) Edges() []ent.Edge { | ||
| return []ent.Edge{ | ||
| edge.From("source_transaction_group", LedgerTransactionGroup.Type). | ||
| Ref("source_breakage_records"). | ||
| Field("source_transaction_group_id"). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.From("source_transaction", LedgerTransaction.Type). | ||
| Ref("source_breakage_records"). | ||
| Field("source_transaction_id"). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.From("source_entry", LedgerEntry.Type). | ||
| Ref("source_breakage_records"). | ||
| Field("source_entry_id"). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.From("breakage_transaction_group", LedgerTransactionGroup.Type). | ||
| Ref("breakage_records"). | ||
| Field("breakage_transaction_group_id"). | ||
| Required(). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.From("breakage_transaction", LedgerTransaction.Type). | ||
| Ref("breakage_records"). | ||
| Field("breakage_transaction_id"). | ||
| Required(). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.From("fbo_sub_account", LedgerSubAccount.Type). | ||
| Ref("fbo_breakage_records"). | ||
| Field("fbo_sub_account_id"). | ||
| Required(). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.From("breakage_sub_account", LedgerSubAccount.Type). | ||
| Ref("breakage_records"). | ||
| Field("breakage_sub_account_id"). | ||
| Required(). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.To("planned_releases", LedgerBreakageRecord.Type). | ||
| Annotations(entsql.OnDelete(entsql.Restrict)), | ||
| edge.To("release_reopens", LedgerBreakageRecord.Type). | ||
| Annotations(entsql.OnDelete(entsql.Restrict)), | ||
| edge.From("plan", LedgerBreakageRecord.Type). | ||
| Ref("planned_releases"). | ||
| Field("plan_id"). | ||
| Immutable(). | ||
| Unique(), | ||
| edge.From("release", LedgerBreakageRecord.Type). | ||
| Ref("release_reopens"). | ||
| Field("release_id"). | ||
| Immutable(). | ||
| Unique(), | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for index declarations covering the FK columns backing the new edges.
fd -e go . openmeter/ent/schema --exec rg -n 'index\.Fields\(' {} \;
echo "---- breakage record indexes ----"
rg -nP 'index\.Fields\([^)]*(source_transaction_id|breakage_transaction_id|fbo_sub_account_id|breakage_sub_account_id|release_id)' openmeter/ent/schema/ledger_breakage_record.go || echo "no matching FK-column indexes found in schema"Repository: openmeterio/openmeter
Length of output: 8239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== schema outline =="
ast-grep outline openmeter/ent/schema/ledger_breakage_record.go --view expanded || true
echo
echo "== relevant schema sections =="
sed -n '1,260p' openmeter/ent/schema/ledger_breakage_record.go
echo
echo "== generated references for ledger_breakage_records =="
rg -n "ledger_breakage_records|source_transaction_id|breakage_transaction_id|fbo_sub_account_id|breakage_sub_account_id|release_id|plan_id|source_transaction_group_id|source_entry_id|breakage_transaction_group_id" openmeter/ent -g '!**/*.json' -g '!**/*.snap' || trueRepository: openmeterio/openmeter
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the generated schema for this entity if it exists.
fd -a 'ledger_breakage_record.go' openmeter/ent | sed -n '1,20p'
echo
for f in $(fd -a 'ledger_breakage_record.go' openmeter/ent); do
echo "== $f =="
rg -n "func \(.*\) Indexes\(\)|index\.Fields|Unique\(|source_transaction_id|breakage_transaction_id|fbo_sub_account_id|breakage_sub_account_id|release_id|plan_id|source_transaction_group_id|source_entry_id|breakage_transaction_group_id" "$f" || true
echo
doneRepository: openmeterio/openmeter
Length of output: 1605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '3695,3810p' openmeter/ent/db/migrate/schema.goRepository: openmeterio/openmeter
Length of output: 4879
Add indexes for the remaining breakage FK columns. source_transaction_id, breakage_transaction_id, fbo_sub_account_id, breakage_sub_account_id, and release_id still don't get indexes in the generated ledger_breakage_records schema, so deletes on the parent tables can end up scanning this table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openmeter/ent/schema/ledger_breakage_record.go` around lines 130 - 187, The
LedgerBreakageRecord schema is still missing indexes on several foreign key
fields, causing parent-table deletes to scan this table. Update the Edges
definition in LedgerBreakageRecord to ensure the remaining FK-backed edges,
especially source_transaction, breakage_transaction, fbo_sub_account,
breakage_sub_account, and release, are indexed in the generated schema by adding
the appropriate index annotations or schema indexes tied to those field names.
Source: Path instructions
Comment on lines
+4
to
+6
| -- modify "ledger_customer_accounts" table | ||
| -- Manual post-generation step: keep this foreign key NOT VALID for the same rollout reason as above. | ||
| ALTER TABLE "ledger_customer_accounts" ALTER COLUMN "account_id" TYPE character(26), ADD CONSTRAINT "ledger_customer_accounts_ledger_accounts_customer_accounts" FOREIGN KEY ("account_id") REFERENCES "ledger_accounts" ("id") ON UPDATE NO ACTION ON DELETE NO ACTION NOT VALID; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Does PostgreSQL treat ALTER COLUMN TYPE from varchar to char(n) as a binary-coercible change that avoids a table rewrite?
💡 Result:
PostgreSQL does not treat an ALTER COLUMN TYPE from varchar to char(n) as a binary-coercible change, and it generally requires a table rewrite [1][2][3]. While PostgreSQL can avoid a full table rewrite when changing between certain types that are binary-coercible or are unconstrained domains over the new type (such as changing from varchar(n) to text or increasing the length of a varchar(n)), the conversion between varchar and char(n) is not considered binary-coercible [1][4]. The char(n) type requires that data be padded with spaces to the specified length n, whereas varchar stores only the string itself. Because the internal on-disk representation differs, PostgreSQL must rewrite the table to convert the values to the fixed-length, space-padded format required by char(n) [5][4]. This operation holds an ACCESS EXCLUSIVE lock on the table for the duration of the rewrite, which can cause significant downtime on large tables [4].
Citations:
- 1: https://www.postgresql.org/docs/19/sql-altertable.html
- 2: https://www.postgresql.org/docs/18/sql-altertable.html
- 3: https://www.postgresql.org/docs/16/sql-altertable.html
- 4: https://monpg.app/blog/postgresql-alter-column-type-no-rewrite
- 5: https://stackoverflow.com/questions/79860839/table-and-index-rewrites-when-changing-type-in-postgresql
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql'
echo '--- file ---'
cat -n "$file"
echo
echo '--- surrounding migration files ---'
fd -a '20260706130134_add_ledger_internal_fks.*\.sql$|20260706130134_add_ledger_internal_fks\.up\.sql|20260706130134_add_ledger_internal_fks\.down\.sql' tools/migrate/migrations || trueRepository: openmeterio/openmeter
Length of output: 3004
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql'
wc -l "$file"
sed -n '1,40p' "$file"Repository: openmeterio/openmeter
Length of output: 2803
Split the type change out from the FK add
ALTER COLUMN "account_id" TYPE character(26) still rewrites ledger_customer_accounts under an ACCESS EXCLUSIVE lock, so the NOT VALID FK doesn’t buy you the low-lock rollout benefit here. Split the type change into its own step.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/migrate/migrations/20260706130134_add_ledger_internal_fks.up.sql`
around lines 4 - 6, The combined ALTER TABLE on ledger_customer_accounts is
still doing a blocking column type rewrite, so separate the account_id type
change from the foreign key addition. Update the migration around the
ledger_customer_accounts ALTER TABLE statement so the TYPE character(26) change
is its own step, then add the
ledger_customer_accounts_ledger_accounts_customer_accounts foreign key in a
separate NOT VALID statement.
Source: Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ledger_customer_accounts.account_idandledger_breakage_recordsledger references.NOT VALIDfor deploy safety.Deploy Callout
After the migration has been applied, run the manual validation commands below after orphan preflight is clean:
Validation
nix develop --impure .#ci -c atlas migrate --env local hashnix develop --impure .#ci -c atlas migrate --env local diff ledger_internal_fks_checknix develop --impure .#ci -c make generatenix develop --impure .#ci -c go vet -tags=dynamic ./openmeter/ledger/...nix develop --impure .#ci -c env POSTGRES_HOST=127.0.0.1 go test -tags=dynamic ./openmeter/ledger/...Summary by CodeRabbit
New Features
Bug Fixes
Tests