Code

Opened 8 years ago

Closed 7 years ago

#3265 closed defect (fixed)

queries for model fields with trailing underscores do not work

Reported by: code.djangoproject@… Owned by: nobody
Component: Database layer (models, ORM) Version:
Severity: trivial Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I've come to the conclusion suffixing an attribute in a model with an underscore is problematic and probably best avoided. It is common practice to suffix reserved keywords with an underscore e.g. select_, print_, etc_ *but* not on an attribute name. These types of attributes should probably cause an exception when trying to syncdb OR the following is a bug:

CharField_ = models.CharField(maxlength=64, unique=True)   # exception: TypeError: Cannot resolve keyword 'CharField' into field
CharField_ = models.CharField(maxlength=64, unique=False)  # everything works fine and no problems are apparent at first

I've discovered the exception inside the Django admin when trying to submit a unique value in the field. It doesn't work if I provide a unique value *but* if the options (blank=True and null=True) is also provided, I can at most submit up to one entry with no value for the field. Every submission thereafter will be sure to cause an exception.

Attachments (0)

Change History (4)

comment:1 Changed 8 years ago by code.djangoproject@…

  • priority changed from normal to low
  • Severity changed from normal to trivial

I've come to the conclusion suffixing an attribute in a model with an underscore is problematic and probably best avoided. It is common practice to suffix reserved keywords with an underscore e.g. select_, print_, etc_ *but* not on an attribute name. These types of attributes should probably cause an exception when trying to syncdb OR the following is a bug:

CharField_ = models.CharField(maxlength=64, unique=True)   # exception: TypeError: Cannot resolve keyword 'CharField' into field
CharField_ = models.CharField(maxlength=64, unique=False)  # everything works fine and no problems are apparent at first

I've discovered the exception inside the Django admin when trying to submit a unique value in the field. It doesn't work if I provide a unique value *but* if the options (blank=True and null=True) is also provided, I can at most submit up to one entry with no value for the field. Every submission thereafter will be sure to cause an exception.

comment:2 Changed 7 years ago by mir@…

  • Component changed from Admin interface to Database wrapper
  • Summary changed from suffixing model attributes can cause TypeError with unique=True to queries for model fields with trailing underscores do not work
  • Triage Stage changed from Unreviewed to Accepted

With unique=True, django tries to find out whether there's a conflicting entry in the database and executes

Testmodel.objects.get(CharField___exact=value)

This fails because the three underscores confuse QuerySet._get_sql_clause(). This can probably be fixed and doesn't need to be sorted out during manage.py validate. It's more or less a lucky coincident that this bug does not hit with unique=False.

I reproduced this with the following model:

from django.db import models

class TestModel(models.Model):
  CharField_ = models.CharField(maxlength=64, unique=True)

  class Admin:
    pass

Then I went into the admin interface and tried to insert anything for this model.

Here's the traceback:

 Traceback (most recent call last):
 File "/home/mir/lib/python/django/core/handlers/base.py" in get_response
   74. response = callback(request, *callback_args, **callback_kwargs)
 File "/home/mir/lib/python/django/contrib/admin/views/decorators.py" in _checklogin
   55. return view_func(request, *args, **kwargs)
 File "/home/mir/lib/python/django/views/decorators/cache.py" in _wrapped_view_func
   39. response = view_func(request, *args, **kwargs)
 File "/home/mir/lib/python/django/contrib/admin/views/main.py" in add_stage
   250. errors = manipulator.get_validation_errors(new_data)
 File "/home/mir/lib/python/django/forms/__init__.py" in get_validation_errors
   59. errors.update(field.get_validation_errors(new_data))
 File "/home/mir/lib/python/django/forms/__init__.py" in get_validation_errors
   352. self.run_validator(new_data, validator)
 File "/home/mir/lib/python/django/forms/__init__.py" in run_validator
   342. validator(new_data.get(self.field_name, ''), new_data)
 File "/home/mir/lib/python/django/db/models/fields/__init__.py" in manipulator_validator_unique
   36. old_obj = self.manager.get(**{lookup_type: field_data})
------------------------------------------------------------
 File "/home/mir/lib/python/django/db/models/manager.py" in get
   67. return self.get_query_set().get(*args, **kwargs)
  ----> with kwargs == {'CharField___exact': 'rarara'}
-------------------------------------------------------------
 File "/home/mir/lib/python/django/db/models/query.py" in get
   211. obj_list = list(clone)
 File "/home/mir/lib/python/django/db/models/query.py" in __iter__
   103. return iter(self._get_data())
 File "/home/mir/lib/python/django/db/models/query.py" in _get_data
   430. self._result_cache = list(self.iterator())
 File "/home/mir/lib/python/django/db/models/query.py" in iterator
   171. select, sql, params = self._get_sql_clause()
 File "/home/mir/lib/python/django/db/models/query.py" in _get_sql_clause
   444. joins2, where2, params2 = self._filters.get_sql(opts)
 File "/home/mir/lib/python/django/db/models/query.py" in get_sql
   574. joins2, where2, params2 = val.get_sql(opts)
 File "/home/mir/lib/python/django/db/models/query.py" in get_sql
   622. return parse_lookup(self.kwargs.items(), opts)
 File "/home/mir/lib/python/django/db/models/query.py" in parse_lookup
   735. joins2, where2, params2 = lookup_inner(path, lookup_type, value, opts, opts.db_table, None)
 File "/home/mir/lib/python/django/db/models/query.py" in lookup_inner
   836. raise TypeError, "Cannot resolve keyword '%s' into field" % name

comment:3 Changed 7 years ago by mtredinnick

It's a fairly expensive computation in query construction to be able to handle trailing underscores like this. Particularly because both "foo" and "foo_" are possible, so we have to try them both/all (what about "foo__", etc?).

Correct fix here is to make trailing underscores a validation error for any Field subclass.

comment:4 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [6590]) Fixed #3265 -- Made it a validation error to have field names with trailing
underscores. Allowing these would enable peopleto write ambiguous queryset
filters (plus makes parsing filters much harder).

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.