Opened 8 years ago

Closed 8 years ago

#23624 closed New feature (wontfix)

Regression in ManyToManyField with through, runtime-generated models

Reported by: Ludovico Magnocavallo Owned by: nobody
Component: Documentation Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Ludovico Magnocavallo)

I admit my situation is a corner case: an app interfacing with a legacy db, where I need to extend an abstract model to contextualize it to identical tables, named differently (eg obj_1 table, obj_2 table, etc.), and to make things worse the number of tables is only known at runtime.

I am then dynamically generating a module for each set of models, and while things work great in Django 1.6, in 1.7 ManyToManyField using a through model breaks.

I am attaching a sample models.py file that illustrates the issue. I have not yet found the time to delve into what's happening, I hope someone with a better understanding of all the related stuff steps in as it's something I have never looked into.

What happens is that the manager seems to ignore the through model, here is the traceback (actual call and models in the attached file):

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/manager.py", line 191, in all
    return self.get_queryset()
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 887, in get_queryset
    return qs._next_is_sticky().filter(**self.core_filters)
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/query.py", line 691, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/query.py", line 709, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1287, in add_q
    clause, require_inner = self._add_q(where_part, self.used_aliases)
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1314, in _add_q
    current_negated=current_negated, connector=connector)
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1138, in build_filter
    lookups, parts, reffed_aggregate = self.solve_lookup_type(arg)
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1076, in solve_lookup_type
    _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta())
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1383, in names_to_path
    self.raise_field_error(opts, name)
  File "/home/ludo/Desktop/dev/venv/spritz/local/lib/python2.7/site-packages/django/db/models/sql/query.py", line 1389, in raise_field_error
    "Choices are: %s" % (name, ", ".join(available)))
FieldError: Cannot resolve keyword u'a' into field. Choices are: a_s, id, name

Attachments (3)

models.py (4.9 KB) - added by Ludovico Magnocavallo 8 years ago.
models.py
django_test.tar.gz (3.9 KB) - added by Ludovico Magnocavallo 8 years ago.
Sample apps for 1.6 and 1.7 with a management command that triggers the bug in 1.7
models.2.py (3.4 KB) - added by Ludovico Magnocavallo 8 years ago.
models.py from the test project that does not trigger the exception

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by Ludovico Magnocavallo

Attachment: models.py added

models.py

comment:1 Changed 8 years ago by Collin Anderson

A trace back would also help, or a failing test-case.

Last edited 8 years ago by Collin Anderson (previous) (diff)

comment:2 Changed 8 years ago by Ludovico Magnocavallo

Description: modified (diff)

comment:3 Changed 8 years ago by Ludovico Magnocavallo

Right, I put the traceback in a comment in the attached file but did not think of putting it here, fixed that now.

Changed 8 years ago by Ludovico Magnocavallo

Attachment: django_test.tar.gz added

Sample apps for 1.6 and 1.7 with a management command that triggers the bug in 1.7

comment:4 Changed 8 years ago by Ludovico Magnocavallo

Progress.

I have found out that the exception disappears if the m2m field name is the same as the fk field name in the through object: the keys in the _name_map of the model holding the m2m field are populated using the model's field names, but when looked up at query time they use the through model's fk field name.

Last edited 8 years ago by Ludovico Magnocavallo (previous) (diff)

comment:5 Changed 8 years ago by Baptiste Mispelon

I've bisected the error to this commit: 9f13c3328199d2fa70235cdc63bb06b1efc5b117

Note that using Python3, you get a slightly more useful error message:

Traceback (most recent call last):
  File "./django/db/models/fields/related.py", line 876, in get_queryset
    return self.instance._prefetched_objects_cache[self.prefetch_cache_name]
AttributeError: 'A' object has no attribute '_prefetched_objects_cache'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "t.py", line 20, in <module>
    a.b_s.all() # works in 1.6, raises FieldError in 1.7
  File "./django/db/models/manager.py", line 191, in all
    return self.get_queryset()
  File "./django/db/models/fields/related.py", line 882, in get_queryset
    return qs._next_is_sticky().filter(**self.core_filters)
  File "./django/db/models/query.py", line 691, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "./django/db/models/query.py", line 709, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "./django/db/models/sql/query.py", line 1287, in add_q
    clause, require_inner = self._add_q(where_part, self.used_aliases)
  File "./django/db/models/sql/query.py", line 1314, in _add_q
    current_negated=current_negated, connector=connector)
  File "./django/db/models/sql/query.py", line 1138, in build_filter
    lookups, parts, reffed_aggregate = self.solve_lookup_type(arg)
  File "./django/db/models/sql/query.py", line 1076, in solve_lookup_type
    _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta())
  File "./django/db/models/sql/query.py", line 1383, in names_to_path
    self.raise_field_error(opts, name)
  File "./django/db/models/sql/query.py", line 1389, in raise_field_error
    "Choices are: %s" % (name, ", ".join(available)))
django.core.exceptions.FieldError: Cannot resolve keyword 'a' into field. Choices are: a_s, id, name

(another interesting thing is that while bisecting, I encountered a different error and even a RecursionError).

Still not sure if this is a bug in Django or if your use-case is just not supported. It certainly looks weird.

comment:6 Changed 8 years ago by Aymeric Augustin

When you suspect a regression caused by the app-loading refactor, I recommend running under -Wall or -Werror.

Django sends warnings in cases that will become errors to make the transition slightly smoother. Unfortunately, since PendingDeprecationWarnings are silent by default, this makes it a bit difficult to pinpoint the root cause ofo some errors.

comment:7 in reply to:  5 Changed 8 years ago by Ludovico Magnocavallo

Replying to bmispelon:

I've bisected the error to this commit: 9f13c3328199d2fa70235cdc63bb06b1efc5b117

Fantastic. The commit points to the solution: a run-time generated models module does not belong to any app, so its models are not used to resolve complex relations.

Creating an AppConfig instance at runtime with the correct name, and a module with a 'models' attribute pointing to the run-time generated models module is not enough though, as one also needs to do some of the stuff that gets done in the app registry's populate method when it's initialized. It's a few lines of code but it feels weird and unclean, it would be ideal if the apps registry had a way to register apps after it has been initialized.

Still not sure if this is a bug in Django or if your use-case is just not supported. It certainly looks weird.

I admit the use-case is a bit specialized, but it's something that used to work in 1.6, and it can be easily made to work in 1.7 with the steps outlined above. The only problem I see is that finding *how* it works is almost impossible for a regular person (it took your involvement to pinpoint the correct commit, etc.). Having a way to register dynamically generated apps/models, and somewhere in the docs that states how to do it would be enough for the few people needing it, I guess.

BTW, my use-case is the Wordpress multi-blog database (each blog has its own set of tables, etc), so it's probably a db design used in lots of other legacy databases.

Changed 8 years ago by Ludovico Magnocavallo

Attachment: models.2.py added

models.py from the test project that does not trigger the exception

comment:8 Changed 8 years ago by Tim Graham

Component: Database layer (models, ORM)Documentation
Resolution: wontfix
Status: newclosed
Type: BugNew feature

I'm going to mark as "won't fix" as there is no official support for runtime-generated models as far as I know.

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