Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#11936 closed (fixed)

Proxy models' names are too long for django.contrib.auth which causes weird test failures

Reported by: ryszard Owned by: Charlie La Mothe
Component: Database layer (models, ORM) Version: 1.1
Severity: Keywords: test, only, deferred fields
Cc: ryszard.szopa+django@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When you call SomeModel.objects.only('somefield'), then the generated deferred proxy model class name may be too long for django.contrib.auth.models.Permission, resulting in DB warnings which are raised as an exception.

This manifests itself if you use only in your tests cases. The just created proxy model will still be floating among the normal models and contrib.auth.management.create_permissions will try to create Permission objects, resulting in queries like

INSERT INTO auth_permission (name, content_type_id, codename) VALUES (Can add episode_ deferred_air_date_art_large_art_medium_art_small_created_description_duration_free_available_free_channel_free_video_url_info_modified_number_parental_rating_released_season_streaming_available_subscription_id, 19, add_episode_deferred_air_date_art_large_art_medium_art_small_created_description_duration_free_available_free_channel_free_video_url_info_modified_number_parental_rating_released_season_streaming_available_subscription_id)

which will cause the NEXT test case to fail.

I was able to reproduce this on Linux, with Python 2.6, MySQL_python 1.2.3c1 and MySQL 5.0.51a (it may depend on how django reacts to MySQL warnings). I will attach a models.py and tests.py that allow to reproduce this bug.

Potential solutions: django.db.models.get_models should not return proxies for deferred models, the deferred proxy models should have shorter names or their type some custom repr.

Attachments (4)

models.py (906 bytes ) - added by ryszard 14 years ago.
models file to trigger the bug
tests.py (498 bytes ) - added by ryszard 14 years ago.
tests to trigger the bug
fix_r11689.diff (2.4 KB ) - added by Charlie La Mothe 14 years ago.
Fix and regression test against r11689
fix_r11901.diff (2.3 KB ) - added by Charlie La Mothe 14 years ago.
Fix and regression test against r11901

Download all attachments as: .zip

Change History (11)

by ryszard, 14 years ago

Attachment: models.py added

models file to trigger the bug

by ryszard, 14 years ago

Attachment: tests.py added

tests to trigger the bug

comment:2 by ryszard, 14 years ago

Cc: ryszard.szopa+django@… added

comment:3 by Karen Tracey, 14 years ago

Reposting previously posted non-formatted traceback. PLEASE use preview and make sure things are readable before pressing "submit changes."

This had been posted by ryszard:

The error that results from runnign the tests with the attached models

(note that test_bbb fails, even though it contains only a pass
statement!).

 (ryszard hacksaw):~/setjam/setjam% ./manage.py test pathology
 Creating test database...
 Creating table pathology_pathologicallylongname
 Creating table auth_permission
 Creating table auth_group
 Creating table auth_user
 Creating table auth_message
 Creating table django_content_type
 Creating table django_session
 Creating table django_site
 Creating table django_admin_log
 Creating table django_flatpage
 Installing index for auth.Permission model
 Installing index for auth.Message model
 Installing index for admin.LogEntry model
 Installing index for flatpages.FlatPage model

 .E

  ======================================================================
 ERROR: test_bbb (pathology.tests.SimpleTest)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
  File "/home/ryszard/setjam/site-python/django/test/testcases.py", line
 242, in __call__
    self._pre_setup()
  File "/home/ryszard/setjam/site-python/django/test/testcases.py", line
 217, in _pre_setup
    self._fixture_setup()
  File "/home/ryszard/setjam/site-python/django/test/testcases.py", line
 440, in _fixture_setup
    return super(TestCase, self)._fixture_setup()
  File "/home/ryszard/setjam/site-python/django/test/testcases.py", line
 222, in _fixture_setup
    call_command('flush', verbosity=0, interactive=False)
  File "/home/ryszard/setjam/site-
 python/django/core/management/__init__.py", line 166, in call_command
    return klass.execute(*args, **defaults)
  File "/home/ryszard/setjam/site-python/django/core/management/base.py",
 line 222, in execute
    output = self.handle(*args, **options)
  File "/home/ryszard/setjam/site-python/django/core/management/base.py",
 line 351, in handle
    return self.handle_noargs(**options)
  File "/home/ryszard/setjam/site-
 python/django/core/management/commands/flush.py", line 61, in
 handle_noargs
    emit_post_sync_signal(models.get_models(), verbosity, interactive)
  File "/home/ryszard/setjam/site-python/django/core/management/sql.py",
 line 205, in emit_post_sync_signal
    interactive=interactive)
  File "/home/ryszard/setjam/site-python/django/dispatch/dispatcher.py",
 line 166, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/home/ryszard/setjam/site-
 python/django/contrib/auth/management/__init__.py", line 28, in
 create_permissions
    defaults={'name': name, 'content_type': ctype})
  File "/home/ryszard/setjam/site-python/django/db/models/manager.py",
 line 123, in get_or_create
    return self.get_query_set().get_or_create(**kwargs)
  File "/home/ryszard/setjam/site-python/django/db/models/query.py", line
 335, in get_or_create
    obj.save(force_insert=True)
  File "/home/ryszard/setjam/site-python/django/db/models/base.py", line
 410, in save
    self.save_base(force_insert=force_insert, force_update=force_update)
  File "/home/ryszard/setjam/site-python/django/db/models/base.py", line
 495, in save_base
    result = manager._insert(values, return_id=update_pk)
  File "/home/ryszard/setjam/site-python/django/db/models/manager.py",
 line 177, in _insert
    return insert_query(self.model, values, **kwargs)
  File "/home/ryszard/setjam/site-python/django/db/models/query.py", line
 1087, in insert_query
    return query.execute_sql(return_id)
  File "/home/ryszard/setjam/site-
 python/django/db/models/sql/subqueries.py", line 320, in execute_sql
    cursor = super(InsertQuery, self).execute_sql(None)
  File "/home/ryszard/setjam/site-python/django/db/models/sql/query.py",
 line 2369, in execute_sql
    cursor.execute(sql, params)
  File "/home/ryszard/setjam/site-
 python/django/db/backends/mysql/base.py", line 84, in execute
    return self.cursor.execute(query, args)
  File "/home/buildbot/bbenv/lib/python2.6/site-
 packages/MySQL_python-1.2.3c1-py2.6-linux-i686.egg/MySQLdb/cursors.py",
 line 175, in execute
    if not self._defer_warnings: self._warning_check()
  File "/home/buildbot/bbenv/lib/python2.6/site-
 packages/MySQL_python-1.2.3c1-py2.6-linux-i686.egg/MySQLdb/cursors.py",
 line 89, in _warning_check
    warn(w[-1], self.Warning, 3)
 Warning: Data truncated for column 'name' at row 1

 ----------------------------------------------------------------------
 Ran 1 test in 0.621s

 FAILED (errors=1)
 Destroying test database...

comment:4 by Charlie La Mothe, 14 years ago

Has patch: set
Owner: changed from nobody to Charlie La Mothe

I've attached a patch that solves the problem for me. The patch applies cleanly against r11689, includes a regression test, and does not break the existing test suite. The included regression test fails if the patch to loading.py is not applied.

With the patch, the django.db.models.loading.get_models method will no longer return any deferred models.

The patch doesn't change get_model, so the deferred models can still be accessed via the app cache if they're individually looked up (as with ModelBase.new).

by Charlie La Mothe, 14 years ago

Attachment: fix_r11689.diff added

Fix and regression test against r11689

comment:5 by b1tr0t, 14 years ago

clamothe: Are you sure that's the correct patch you added? At first glance it doesn't look related to the problem, and certainly it doesn't fix the bug for me on 1.2 pre-alpha. :)

comment:6 by Charlie La Mothe, 14 years ago

Yes, I added the correct patch. I will explain how it fixes the problem for me.

The problem

If the following sounds like gibberish then read the background info first.

In the aftermath of a query that used QuerySet's defer() or only() methods, a deferred model could exist. Without my patch, the results of the django.db.models.loading.get_models method include deferred models. With my patch, the results of this method do not include deferred models.

If deferred models are included in the results of this method then they could be mistaken for real models. An example of of such is when django.contrib.auth autopopulates it's permission table. Certainly creating separate permissions for deferred models is not intended. Additionally, the name of the deferred model's class might not fit in the 50 characters allowed in Permission's name field, causing a database error or warning such as:

 Warning: Data truncated for column 'name' at row 1

Background information

When QuerySet's defer() or only() methods are used in a query, django dynamically creates a model that includes only the fields that the QuerySet needs. If User.objects.only('username', 'password') is called then django will create a special deferred User model that only includes the username and password fields. Django would call this model User_Deferred_username_password. If a model has many fields and one or two fields are deferred in a query then the name of the deferred model created for the queryset can be very long, as it includes the names of each field that will be retrieved.

Keep that in mind while we jump over to django.contrib.auth. This app creates the permissions for each installed model whenever the database is reset or syncdb is called. Remember the model permissions that you can assign to Users and Groups when using the admin? This is where those permissions are created. The method that creates these permissions first looks up each installed model, using django.db.models.loading.get_models to get the installed models, then creates the Permission objects for each installed model. Each Permission object is assigned a name. The name field contains a description of the permission (i.e. "Can add Group") and has a max_length of 50.

by Charlie La Mothe, 14 years ago

Attachment: fix_r11901.diff added

Fix and regression test against r11901

comment:7 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: newclosed

(In [11938]) Fixed #11936 -- Removed deferred models from the list returned by the app_cache. Thanks to ryszard for the report, and clamothe for the initial patch.

comment:8 by Russell Keith-Magee, 14 years ago

(In [11942]) [1.1.X] Fixed #11936 -- Removed deferred models from the list returned by the app_cache. Thanks to ryszard for the report, and clamothe for the initial patch.

Backport of r11938 from trunk.

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