Opened 4 weeks ago

Closed 4 weeks ago

Last modified 4 weeks ago

#35453 closed Bug (invalid)

ManyToMany field is a concrete field on the defining side.

Reported by: Harro Owned by: nobody
Component: Database layer (models, ORM) Version: 5.0
Severity: Normal Keywords:
Cc: Mariusz Felisiak, Simon Charette Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Was looking at some relationship bugs in: https://github.com/bmispelon/django-model-subquery
And when trying to fix it ran into the following problem:

ManyToMany fields are concrete and have a column on the defining side.

Take the User in django, if I do:

[f.column for f in User._meta.get_fields() if f.concrete]

You see groups in there, but it's a ManyToMany so there is no actual column called groups in the user table.

Then I dove into django and actually see django itself use local_concrete_fields internally a lot, which has all the actual fields that have a column.

Shouldn't the ManyToMany basically not be a concrete field so it actually matches what you expect and what the docs say about the field property:
https://docs.djangoproject.com/en/5.0/ref/models/fields/#django.db.models.Field.concrete

Change History (5)

comment:1 by Sarah Boyce, 4 weeks ago

Cc: Mariusz Felisiak Simon Charette added
Resolution: invalid
Status: newclosed

I think the docs could potentially be clearer but I don't think ManyToManyField should have concrete as False.
I believe local_concrete_fields is when the column exists on the model's table (rather than on a different table) and so is "local".
I think the docs are correct that ManyToManyField does have a database column just not locally. Examples of non-concrete fields include GenericForeignKey, GenericRelation and ForeignObject.

comment:2 by Harro, 4 weeks ago

Which column does a ManyToMany have then?

As far as I see it it has a table with has two (or more) columns pointing to two different tables, so why doesn't the other side of the ManyToMany not also have a column?

local_concrete_fields is local_fields filtered on the concrete flag, but I can't find where the local_fields is filled in the source code, but it does not contain the ManyToMany fields defined on the model (so far that's correct)

Also, the documentation doesn't mention the local_fields nor the local_concrete_fields so techically they shouldn't be used, but the get_field(s) api calls are documented, so I expect it to give me the information I need, as to which models are actually on a database table without extra weird conditions to exclude specifically one side of the ManyToMany relation.

comment:3 by Harro, 4 weeks ago

I tried adding

def get_attname_column(self):
        attname, column = super().get_attname_column()
        return attname, None

to the ManyToMany field and it broke 7 tests that all seem to be related to migrations/schema, nothing else.

So somewhere internally django migrations depend on ManyToMany having a column, which isn't actually there at all.

in reply to:  2 comment:4 by Sarah Boyce, 4 weeks ago

Replying to Harro:

Also, the documentation doesn't mention the local_fields nor the local_concrete_fields so techically they shouldn't be used, but the get_field(s) api calls are documented, so I expect it to give me the information I need, as to which models are actually on a database table without extra weird conditions to exclude specifically one side of the ManyToMany relation.

Yes, these are not documented. Previous work to document Model _meta API was done as part of #12663.
I don't think using local_fields is a big risk and you could use this, maybe there is a case for it being documented.

comment:5 by Harro, 4 weeks ago

Yeah, maybe that would solve it, get_fields seems to have some caching but still does a lot of processing, while local_fields is stored on the options and the others are cached properties that only loop through the fields once.

So performance wise it might be better to use them anyway.

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