ManyToMany field incorrectly identified as a concrete field on the defining side.

[Harro]

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

[Sarah Boyce]

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.

[Harro]

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.

[Harro]

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.

[Sarah Boyce]

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.

[Harro]

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.

[Ryan P Kilby]

I believe this ticket should be reopened. Related to https://code.djangoproject.com/ticket/36481 and the discussion in ​https://github.com/django/django/pull/19595, @charettes pointed out that ManyToManyField is likely incorrectly identified as concrete and that it should be investigated.

[Simon Charette]

FWIW the only tests failing when applying

django/db/models/fields/related.py

@@ -2062 +2062 @@
 
     def db_parameters(self, connection):
         return {"type": None, "check": None}
+
+    def get_attname_column(self):
+        attname, column = super().get_attname_column()
+        return attname, None

Are migrations related one mainly because we skip non-concrete fields alterations (see _field_should_be_altered) which can be further addressed by

django/db/backends/base/schema.py

@@ -204 +204 @@
                 cursor.execute(sql, params)
 
     def quote_name(self, name):
+        if name is None:
+            return name
         return self.connection.ops.quote_name(name)
 
     def table_sql(self, model):
@@ -1655 +1657 @@
         return output
 
     def _field_should_be_altered(self, old_field, new_field, ignore=None):
-        if not old_field.concrete and not new_field.concrete:
+        if not (
+            old_field.concrete or old_field.is_relation and old_field.many_to_many
+        ) and not (
+            new_field.concrete or new_field.is_relation and new_field.many_to_many
+        ):
             return False
         ignore = ignore or set()
         _, old_path, old_args, old_kwargs = old_field.deconstruct()

I think it's worth discussing what we want to do with non-concrete field alterations in general as it has some implications with composite foreign key support as we'll most likely have to make such foreign keys non-concrete as discussed ticket:35956#comment:13.