Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#12712 closed (fixed)

syncdb doesn't create tables for m2m through-less fields that are added on the model after the class_prepared signal

Reported by: Ionel Cristian Maries Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2-alpha
Severity: Keywords:
Cc: ionel.mc@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

It seems that syncdb adds the auto_created m2m tables only if the table for the model containing the m2m field weren't created yet.

This change was introduced in one of the multidb updates - see http://code.djangoproject.com/changeset/11710#file2

I have a weird use case here: I have a custom field that adds a m2m field on the model on the contribute_to_class call (unfortunately it happens later after the model is prepared due to some information needed from other models coupled with lazy relationships). This looks like a regression to me, 1.1 was handling this case just fine. Also, the m2m fields don't have an explicit through table (thus the auto_created flag).

Reverting back the condition in syncdb seems to solve this problem (also, the testsuite doesn't show any regression for me).

Attachments (2)

syncdb-fix.patch (945 bytes) - added by Ionel Cristian Maries 7 years ago.
django-m2m-syncdb.diff (1.7 KB) - added by Alex Gaynor 7 years ago.

Download all attachments as: .zip

Change History (12)

Changed 7 years ago by Ionel Cristian Maries

Attachment: syncdb-fix.patch added

comment:1 Changed 7 years ago by Ionel Cristian Maries

Has patch: set

comment:2 Changed 7 years ago by Ionel Cristian Maries

Forgot to note this:

The fact that the m2m field is added in the model after the class_prepared signal changes the order in the tables list (tables = connection.introspection.table_names()). So in my scenario the m2m table appears after the table of the model containing the m2m filed.

In a normal scenario the m2m table appears before the table of the model containing the m2m field.

comment:3 Changed 7 years ago by Russell Keith-Magee

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

I can see how the problem you have can arise, but I'm not sure the patch is the right solution. In order to be completely consistent, all m2m tables need to be made after all normal tables.

Changed 7 years ago by Alex Gaynor

Attachment: django-m2m-syncdb.diff added

comment:4 Changed 7 years ago by Alex Gaynor

Patch needs improvement: unset

comment:5 Changed 7 years ago by James Bennett

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:6 Changed 7 years ago by Alex Gaynor

milestone: 1.2

This is a bug, and a regression, not a feature.

comment:7 Changed 7 years ago by Ionel Cristian Maries

Another case that still breaks:

Running syncdb with these patches (any of them) after syncdb-ing an empty database with an unpatched django doesn't create the m2m tables.

comment:8 in reply to:  7 Changed 7 years ago by Russell Keith-Magee

Replying to IonelMaries:

Another case that still breaks:

Running syncdb with these patches (any of them) after syncdb-ing an empty database with an unpatched django doesn't create the m2m tables.

This is to be expected. Even though m2m fields are stored in a separate table, they're not handled any differently to normal fields. If you add a field to an existing table, Django doesn't update the table. Simliarly, if you add an m2m field (or appear to add an m2m field, which is what is happening here), Django doesn't create the m2m table.

comment:9 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [12597]) Fixed #12712 -- Corrected a problem with synchronizing that prevented m2m tables from being created under certain circumstances. Thanks to IonelMaries for the report, and Alex Gaynor for the patch.

comment:10 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top