Default new models to BigAutoField, widening existing integer primary keys#6754
Default new models to BigAutoField, widening existing integer primary keys#6754norkans7 wants to merge 1 commit into
Conversation
… keys Remove the DEFAULT_AUTO_FIELD = AutoField setting so new models use Django's BigAutoField default, and add migrations converting the existing implicit integer primary keys to BigAutoField across all apps. Contact is deliberately kept on an explicit integer AutoField to avoid rewriting the very large contacts_contact table and the contact_id foreign key columns on high-volume tables. In addition, the org/flow/user primary-key conversions keep contacts_contact's own foreign key columns (org_id, current_flow_id, created_by_id, modified_by_id) as integer: Postgres allows an integer foreign key column to reference a bigint primary key, so these constraints are dropped and re-added without widening the columns, leaving contacts_contact untouched (no table rewrite). This is done via SeparateDatabaseAndState so Django's model state still treats the columns as bigint and makemigrations stays clean. Also update the contact_toggle_system_group SQL function to accept bigint arguments now that org_id is bigint.
|
Claude finished @norkans7's task in 9m 34s —— View job Claude PR ReviewReviewed the migration set, Blocking / verify-before-merge
Non-blocking
Nit — the wrapper
|
| SET CONSTRAINTS "locations_boundaryalias_org_id_930a8491_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "locations_boundaryalias" DROP CONSTRAINT "locations_boundaryalias_org_id_930a8491_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "ivr_call_org_id_f05ab39d_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "ivr_call" DROP CONSTRAINT "ivr_call_org_id_f05ab39d_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "channels_channelevent_org_id_4d7fff63_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "channels_channelevent" DROP CONSTRAINT "channels_channelevent_org_id_4d7fff63_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "channels_channel_org_id_fd34a95a_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "channels_channel" DROP CONSTRAINT "channels_channel_org_id_fd34a95a_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "orgs_org_parent_id_79ba1bbf_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "orgs_org" DROP CONSTRAINT "orgs_org_parent_id_79ba1bbf_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "contacts_contactimport_org_id_b7820a48_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "contacts_contactimport" DROP CONSTRAINT "contacts_contactimport_org_id_b7820a48_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "contacts_contactfire_org_id_9dc80e0c_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "contacts_contactfire" DROP CONSTRAINT "contacts_contactfire_org_id_9dc80e0c_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "contacts_contact_org_id_01d86aa4_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "contacts_contact" DROP CONSTRAINT "contacts_contact_org_id_01d86aa4_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "orgs_itemcount_org_id_604bb8db_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "orgs_itemcount" DROP CONSTRAINT "orgs_itemcount_org_id_604bb8db_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "orgs_dailycount_org_id_e73b93d3_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "orgs_dailycount" DROP CONSTRAINT "orgs_dailycount_org_id_e73b93d3_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "orgs_export_org_id_3d9eebc7_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "orgs_export" DROP CONSTRAINT "orgs_export_org_id_3d9eebc7_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "contacts_contactfield_org_id_d83cc86a_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "contacts_contactfield" DROP CONSTRAINT "contacts_contactfield_org_id_d83cc86a_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "flows_flowlabel_org_id_4ed2f553_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "flows_flowlabel" DROP CONSTRAINT "flows_flowlabel_org_id_4ed2f553_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "flows_flowstart_org_id_d5383c58_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "flows_flowstart" DROP CONSTRAINT "flows_flowstart_org_id_d5383c58_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "flows_flow_org_id_51b9c589_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "flows_flow" DROP CONSTRAINT "flows_flow_org_id_51b9c589_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "contacts_contactgroup_org_id_be850815_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "contacts_contactgroup" DROP CONSTRAINT "contacts_contactgroup_org_id_be850815_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "orgs_orgimport_org_id_d9346c57_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "orgs_orgimport" DROP CONSTRAINT "orgs_orgimport_org_id_d9346c57_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "notifications_incident_org_id_b5cb2702_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "notifications_incident" DROP CONSTRAINT "notifications_incident_org_id_b5cb2702_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "orgs_invitation_org_id_d9d2be95_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "orgs_invitation" DROP CONSTRAINT "orgs_invitation_org_id_d9d2be95_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "ai_llm_org_id_21c1fd13_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "ai_llm" DROP CONSTRAINT "ai_llm_org_id_21c1fd13_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "notifications_notification_org_id_b4c202e9_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "notifications_notification" DROP CONSTRAINT "notifications_notification_org_id_b4c202e9_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "orgs_orgmembership_org_id_35cc8172_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "orgs_orgmembership" DROP CONSTRAINT "orgs_orgmembership_org_id_35cc8172_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "flows_flowrun_org_id_07d5f694_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "flows_flowrun" DROP CONSTRAINT "flows_flowrun_org_id_07d5f694_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "tickets_shortcut_org_id_5fbbda6f_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "tickets_shortcut" DROP CONSTRAINT "tickets_shortcut_org_id_5fbbda6f_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "tickets_team_org_id_2a011111_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "tickets_team" DROP CONSTRAINT "tickets_team_org_id_2a011111_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "tickets_ticket_org_id_56b25ecf_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "tickets_ticket" DROP CONSTRAINT "tickets_ticket_org_id_56b25ecf_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "tickets_topic_org_id_0b22bd8a_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "tickets_topic" DROP CONSTRAINT "tickets_topic_org_id_0b22bd8a_fk_orgs_org_id"; | ||
| SET CONSTRAINTS "contacts_contacturn_org_id_3cc60a3a_fk_orgs_org_id" IMMEDIATE; ALTER TABLE "contacts_contacturn" DROP CONSTRAINT "contacts_contacturn_org_id_3cc60a3a_fk_orgs_org_id"; | ||
| ALTER TABLE "orgs_org" ALTER COLUMN "id" TYPE bigint USING "id"::bigint; | ||
| ALTER SEQUENCE IF EXISTS "orgs_org_id_seq" AS bigint; | ||
| ALTER TABLE "locations_boundaryalias" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "ivr_call" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "channels_channelevent" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "channels_channel" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "orgs_org" ALTER COLUMN "parent_id" TYPE bigint USING "parent_id"::bigint; | ||
| ALTER TABLE "contacts_contactimport" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "contacts_contactfire" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "orgs_itemcount" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "orgs_dailycount" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "orgs_export" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "contacts_contactfield" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "flows_flowlabel" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "flows_flowstart" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "flows_flow" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "contacts_contactgroup" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "orgs_orgimport" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "notifications_incident" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "orgs_invitation" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "ai_llm" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "notifications_notification" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "orgs_orgmembership" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "flows_flowrun" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "tickets_shortcut" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "tickets_team" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "tickets_ticket" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "tickets_topic" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; | ||
| ALTER TABLE "contacts_contacturn" ALTER COLUMN "org_id" TYPE bigint USING "org_id"::bigint; |
There was a problem hiding this comment.
Forward/reverse asymmetry: msgs_* and schedules_schedule FKs are handled only in the reverse SQL.
The FORWARD block drops the FK constraints for locations_boundaryalias, ivr_call, channels_*, orgs_*, contacts_*, flows_*, tickets_*, notifications_*, ai_llm, etc. before altering orgs_org.id, but omits msgs_broadcast, msgs_media, msgs_msg, msgs_label, msgs_optin, and schedules_schedule — even though each has an FK from org_id to orgs_org.id (see msgs/models.py:63,206,593,1017,1116 and schedules/models.py). The REVERSE block (lines 139–171 / 175–201 / 208–235) does drop and re-add all of them.
Two consequences to verify:
-
Forward may fail on a populated DB. Modern PostgreSQL will usually allow
ALTER COLUMN … TYPE biginton a referenced PK when dependent FK columns stayinteger(the FK is still enforceable via widening cast), but this only holds if the underlying storage rewrite doesn't invalidate the constraint. On a from-scratch (empty) database — as tested per the PR description — the operation is trivial and will succeed regardless. On a real workspace DB with millions of rows inmsgs_msg, the ALTER may take a long lock and, on some versions, be rejected outright with an "incompatible types" error against these constraints. Worth running the forward migration against a restore of production or at least a seeded DB, not just a fresh one. -
Design intent looks inconsistent. The PR description says only
contacts_contactFKs stayinteger; every other referencing column gets widened. But because these sixmsgs_*/schedules_*org_idcolumns aren't touched by the forward SQL, they'll silently remainintegerwhile Django's model state records them asbigint(sinceOrg.idis nowBigAutoField). If the intent was to widen them (matching the description), the forward SQL is missing the drop → widen → re-add blocks. If the intent was to preserve them asintegerto avoid rewritingmsgs_msgin particular, then (a) it should be called out in the migration comment likecontacts_contactis, and (b) the REVERSE SQL should not "narrow" columns that were never widened (theTYPE integer USING ::integeron already-integer columns is a wasteful no-op, and the drop/re-add of their FKs in reverse also seems unnecessary).
The asymmetry alone is a smell — please confirm which behavior was intended and make forward and reverse mirror each other.
| ("sql", "0008_squashed"), | ||
| ] | ||
|
|
||
| operations = [migrations.RunSQL(SQL, reverse_sql=migrations.RunSQL.noop)] |
There was a problem hiding this comment.
reverse_sql=RunSQL.noop leaves the DB with the new BIGINT overload if this migration is unapplied.
If someone reverses to before this migration, the contact_toggle_system_group(INT, INT, …) overload will still be gone (dropped by DROP FUNCTION IF EXISTS in forward) and only the BIGINT version will remain — which is functionally fine (bigint accepts int by implicit cast), but leaves the schema inconsistent with what earlier migrations installed. It also means re-forwarding will hit CREATE OR REPLACE on an already-installed function.
If you care about clean reversibility, the reverse could be the pre-migration function definitions (INT variants). If not, this is acceptable — but worth a one-line comment noting that reversal is intentionally non-restoring.
Also note: this migration depends on ("sql", "0008_squashed") but doesn't depend on orgs/0186 — since _org_id BIGINT in the new function is only used as a filter value, that's fine, but you probably want a hard ordering (orgs.0186 before contacts.0218) so that a partial migrate never leaves a BIGINT-signatured helper being called with an INT _org_id from an old orgs_org.id. Add ("orgs", "0186_alter_export_id_alter_invitation_id_alter_org_id_and_more") to be safe.
| operations = [ | ||
| migrations.AlterField( | ||
| model_name="broadcast", | ||
| name="id", | ||
| field=models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID"), | ||
| ), | ||
| migrations.AlterField( | ||
| model_name="label", | ||
| name="id", | ||
| field=models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID"), | ||
| ), | ||
| migrations.AlterField( | ||
| model_name="media", | ||
| name="id", | ||
| field=models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID"), | ||
| ), | ||
| migrations.AlterField( | ||
| model_name="optin", | ||
| name="id", | ||
| field=models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID"), | ||
| ), | ||
| ] |
There was a problem hiding this comment.
Widening msgs_broadcast.id will rewrite msgs_msg — the largest table in the system.
msgs_msg.broadcast_id is a FK to msgs_broadcast.id (see msgs/models.py:601). This plain AlterField runs through Django's schema editor, which will drop the FK, widen msgs_msg.broadcast_id from integer to bigint, and re-add the FK — forcing a full rewrite of msgs_msg. The same applies to the contacts.0217 (contacturn.id → widens msgs_msg.contact_urn_id) and channels.0211 (channel.id → widens msgs_msg.channel_id) migrations in this PR.
The PR description highlights contacts_contact as the "very large table" being kept from rewrite, but on a production RapidPro msgs_msg is typically comparable or larger than contacts_contact. If the reason for special-casing contacts_contact is table size / migration time, the same reasoning applies to msgs_msg, and the standard AlterField here (plus the ones in channels.0211 and contacts.0217) will each trigger a full msgs_msg rewrite in sequence. Please confirm the operational impact is acceptable, or apply the same SeparateDatabaseAndState + hand-crafted SQL pattern to keep msgs_msg.broadcast_id / channel_id / contact_urn_id as integer.
| STATUS_ARCHIVED: "archived", | ||
| } | ||
|
|
||
| id = models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID") |
There was a problem hiding this comment.
Good — this explicit AutoField is what pins Contact to integer PK after DEFAULT_AUTO_FIELD is removed. One thing worth adding: a short comment on this line explaining why Contact deviates from the project-wide BigAutoField default, so a future contributor doesn't "clean it up" during a refactor without realizing the contacts_contact table would need a full rewrite. e.g.
# kept as int (not BigAutoField) so contacts_contact is not rewritten; see migration 0186/0217/0404/0022
id = models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")
Removes the
DEFAULT_AUTO_FIELD = AutoFieldsetting so new models use Django'sBigAutoFielddefault, and converts the existing implicit integer primary keys toBigAutoFieldacross all apps.Keeping the large
contacts_contacttable from being rewrittenContactstays on an explicit integerAutoField, so thecontacts_contactprimary key and thecontact_idforeign key columns on high-volume tables are not widened.Additionally, converting the
org/flow/userprimary keys would normally widencontacts_contact's own foreign key columns (org_id,current_flow_id,created_by_id,modified_by_id) — a full rewrite of that large table. Postgres allows an integer foreign key column to reference a bigint primary key, so those three migrations instead drop and re-add thecontacts_contactforeign keys without widening the columns (constraint names preserved for reversibility). This is expressed withSeparateDatabaseAndStateso Django's model state still treats the columns as bigint andmakemigrationsstays clean.Result (verified on a from-scratch migrate): all converted parent PKs are
bigint, every referencing column is widened except those pointing atContact, which stayinteger— leavingcontacts_contactuntouched.Also
contact_toggle_system_groupSQL function updated to takebigintarguments now thatorg_idis bigint.Verification
contacts_contactcolumns confirmed to remainintegerwith foreign keys intact and enforcing.makemigrations --check,ruff, andcode_check.pyclean; contacts group tests pass against a freshly migrated DB.Draft: alternative to #6752, which is kept open.