Code

Opened 6 years ago

Closed 2 years ago

#8886 closed Bug (fixed)

loaddata deserializes BooleanFields wrong for inheriting model

Reported by: erichs@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.0
Severity: Normal Keywords: loaddata BooleanField
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using a very simple model of an Employee that extends auth.User, BooleanFields always keep the default value, ignoring the actual value from the JSON file.

Attached is a simple model that extends auth.User and a sample JSON dump. Simply sync the database and load the JSON file. While the boolean values for User have their correct values, the "is_active" field for Employee will be true for both. If you remove the "default=True", both of them will be false, which isn't correct either.

Attachments (2)

models.py (452 bytes) - added by erichs@… 6 years ago.
infrax.json (2.2 KB) - added by erichs@… 6 years ago.

Download all attachments as: .zip

Change History (8)

Changed 6 years ago by erichs@…

Changed 6 years ago by erichs@…

comment:1 Changed 6 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

The problem here will stem from the ambiguity of having a field on the child class that has the same name as a field on the base class. I'm actually surprised that this isn't caught as a validation error in the model definition, but given that it isn't, I can see how this would cause problems for the serializer.

I'm uncertain if the solution here is to raise a validation error for fields with ambiguous names, or to allow this kind of model and correct the serialization. I can see arguments for both, but my gut reaction is that the former is probably the right solution.

comment:2 Changed 6 years ago by mtredinnick

This looks like a bug in model inheritance. There's code in django.db.models.base.ModelBase.__new__ that tries to detect duplicate field names and raises an error for them, although at some point I apparently suffered some kind of blow to the head an only implemented it for abstract base classes. I don't think that was intentional. There are drawbacks to trying to allowed overloading, plus it leads to no end of errors on the developer end. Database model inheritance isnt' a complete match with Python model inheritance and it's trivial to just not use duplicate names one they're reported.

So, don't do that (you can change the model without have to change the database table by using the db_column attribute). It's a bug that that kind of model is legal in the first place.

comment:3 Changed 6 years ago by jdunck

  • Component changed from Serialization to Database layer (models, ORM)

Changing to Database Layer since this appears to be a missing model validation rather than a serialization bug, per Malcolm's response.

comment:4 Changed 4 years ago by anonymous

Will it EVER be fixed? It's annoying. Why shouldn't I have user = model.ForeignKey(User) in the first place? What am I going to call it? user_? user_whatever? Just, wtf.. This should be fixed for 1.2, you know..

comment:5 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

comment:6 Changed 2 years ago by aaugustin

  • Easy pickings unset
  • Resolution set to fixed
  • Status changed from new to closed
  • UI/UX unset

The example provided by the OP now properly raises a ValidationError:

Traceback (most recent call last):
  File "./manage.py", line 9, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/__init__.py", line 422, in execute_from_command_line
    utility.execute()
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/__init__.py", line 361, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/base.py", line 192, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/base.py", line 228, in execute
    output = self.handle(*args, **options)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/base.py", line 364, in handle
    return self.handle_noargs(**options)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/commands/validate.py", line 9, in handle_noargs
    self.validate(display_num_errors=True)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/base.py", line 262, in validate
    num_errors = get_validation_errors(s, app)
  File "/Users/myk/Documents/dev/django-trunk/django/core/management/validation.py", line 30, in get_validation_errors
    for (app_name, error) in get_app_errors().items():
  File "/Users/myk/Documents/dev/django-trunk/django/db/models/loading.py", line 158, in get_app_errors
    self._populate()
  File "/Users/myk/Documents/dev/django-trunk/django/db/models/loading.py", line 64, in _populate
    self.load_app(app_name, True)
  File "/Users/myk/Documents/dev/django-trunk/django/db/models/loading.py", line 88, in load_app
    models = import_module('.models', app_name)
  File "/Users/myk/Documents/dev/django-trunk/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "/Users/myk/Documents/dev/test_project/test_app/models.py", line 22, in <module>
    class Employee(User):
  File "/Users/myk/Documents/dev/django-trunk/django/db/models/base.py", line 149, in __new__
    (field.name, name, base.__name__))
django.core.exceptions.FieldError: Local field 'is_active' in class 'Employee' clashes with field of similar name from base class 'User'

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.