Opened 18 years ago

Closed 12 years ago

#1751 closed defect (fixed)

unique_together does not work when any of the listed fields contains a FK

Reported by: Olive Owned by: Gary Wilson
Component: contrib.admin Version: 1.3
Severity: normal Keywords: fk unique_together
Cc: nslater@…, gary.wilson@…, cathy@…, larlet@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

An IntegrityError is raised when saving the following kind of object:

class DocTitle(models.Model):
    sequence    = models.IntegerField(_('sequence'))
    title_en    = models.CharField(_('title'),maxlength=200)
    
    def __repr__(self):
        return self.title_en
    
    class Meta:
        verbose_name = _('documentation title')
        verbose_name_plural = _('documentation titles')
        ordering = ['sequence']
        
    class Admin:
        list_display = ('title_en',)
             
class DocChunk(models.Model):
    application = models.ForeignKey(Application,edit_inline=True,num_in_admin=1)
    title       = models.ForeignKey(DocTitle)
    text        = models.TextField(_('text'),core=True)
    
    def __repr__(self):
        return self.title.title_en
    
    class Meta:
        verbose_name = _('Document chunk')
        verbose_name_plural = _('Document chunks')
        ordering = ['title']
        unique_together = (("application","title"),)

Change History (33)

comment:1 by Olive, 18 years ago

I mean, when saving an "application" object, of course

comment:2 by Olive, 18 years ago

Severity: majorblocker

In fact the problem occures if there is at least one ForeignKey field specified in unique_together list (no matter if it is an edit_inline or not)

comment:3 by Adrian Holovaty, 18 years ago

priority: highnormal
Severity: blockernormal

comment:4 by anonymous, 18 years ago

Yeah, what's the deal with this?

comment:5 by anonymous, 18 years ago

Summary: unique_together does not work when one of the fields has edit_inline set to Trueunique_together does not work when any of the fields contains a FK or has edit_inline set to True

comment:6 by anonymous, 18 years ago

Summary: unique_together does not work when any of the fields contains a FK or has edit_inline set to Trueunique_together does not work when any of the listed fields contains a FK

comment:7 by Gary Wilson <gary.wilson@…>, 18 years ago

Cc: gary.wilson@… added

I am using unique_together with ForeignKey fields in a model that looks like:

class Course(models.Model):
    """A course number and title combination."""
    
    department = models.ForeignKey(Department)
    number = models.CharField('course number', maxlength=6)
    title = models.CharField('course title', maxlength=100)
    
    class Meta:
        # Don't allow duplicate courses.
        unique_together = (("department", "number", "title"),)

I have no problems saving Department or Course objects. I also have no problems when I add edit_inline=True in the department field as long as at least one other field has core=True. It's only if I fail to add a core=True to any field that I get the following IntegrityError:

Traceback (most recent call last):
File "/usr/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  74. response = callback(request, *callback_args, **callback_kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/usr/lib/python2.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in add_stage
  254. new_object = manipulator.save(new_data)
File "/usr/lib/python2.4/site-packages/django/db/models/manipulators.py" in save
  198. new_rel_obj.save()
File "/usr/lib/python2.4/site-packages/django/db/models/base.py" in save
  203. ','.join(placeholders)), db_values)
File "/usr/lib/python2.4/site-packages/django/db/backends/util.py" in execute
  12. return self.cursor.execute(sql, params)

  IntegrityError at /admin/courses/department/add/
  ERROR: duplicate key violates unique constraint "courses_course_department_id_key" INSERT INTO "courses_course" ("department_id","number","title") VALUES (6,'','')

I also have a model that uses unique_together using two fields that are both ForeignKeys and I don't experience any problem there with or without edit_inline=True.

comment:8 by favo@…, 18 years ago

Notice that "This validation needs to occur after html2python to be effective." but validator always run before html2python. so I change ForeignKey to use field.name instead of field.attname.

Index: db/models/manipulators.py
===================================================================
--- db/models/manipulators.py   (revision 4643)
+++ db/models/manipulators.py   (working copy)
@@ -283,7 +283,7 @@
         # This is really not going to work for fields that have different
         # form fields, e.g. DateTime.
         # This validation needs to occur after html2python to be effective.
-        field_val = all_data.get(f.attname, None)
+        field_val = all_data.get(f.name, None)
         if field_val is None:
             # This will be caught by another validator, assuming the field
             # doesn't have blank=True.

comment:9 by favo@…, 18 years ago

New version: unique together fields could be null=True now.

Index: db/models/manipulators.py
===================================================================
--- db/models/manipulators.py   (revision 4643)
+++ db/models/manipulators.py   (working copy)
@@ -283,13 +283,13 @@
         # This is really not going to work for fields that have different
         # form fields, e.g. DateTime.
         # This validation needs to occur after html2python to be effective.
-        field_val = all_data.get(f.attname, None)
-        if field_val is None:
+        field_val = f.get_manipulator_new_data(all_data)
+        if not f.null and field_val is None:
             # This will be caught by another validator, assuming the field
             # doesn't have blank=True.
             return
         if isinstance(f.rel, ManyToOneRel):
-            kwargs['%s__pk' % f.name] = field_val
+            kwargs[f.get_validator_unique_lookup_type()] = field_val
         else:
             kwargs['%s__iexact' % f.name] = field_val
     try:

comment:10 by anonymous, 17 years ago

Cc: cathy@… added

comment:11 by Noah Slater, 17 years ago

Cc: nslater@… added

I can confirm that the first patch submitted by favo works.

comment:12 by Gary Wilson <gary.wilson@…>, 17 years ago

Maybe the problem is backend specific? For anyone seeing this problem, which database backend are you using?

My comments from above were on postgresql.

comment:13 by anonymous, 17 years ago

Same here, PostgreSQL.

comment:14 by Cathy Young, 17 years ago

I was on MySQL when I hit this problem.

in reply to:  14 comment:15 by anonymous, 17 years ago

I had this same problem until I switched the order of my fields in the unique_together clause. In my example, using 'slug' first threw an integrity error, but having 'site' first validates correctly. (I'm working in 91-bugfixes with PostgreSQL)

 slug = meta.SlugField(prepopulate_from=('name',))
 site = meta.ForeignKey(Site)

 class META:
     unique_together=(('site', 'slug'),)

comment:16 by Simon G. <dev@…>, 17 years ago

Keywords: fk unique_together added
Triage Stage: UnreviewedAccepted

comment:17 by Gary Wilson <gary.wilson@…>, 17 years ago

#2019 marked as a duplicate of this.

in reply to:  8 comment:18 by John Shaffer <jshaffer2112@…>, 17 years ago

Replying to favo@exoweb.net:

I change ForeignKey to use field.name instead of field.attname.

This change was made in [4028].

comment:19 by Gábor Farkas <gabor@…>, 17 years ago

i'm not sure what's the status of this ticket, but maybe a clean reproducible test-case will help.

this is with from-svn-django (r6022), postgresql8, "postgresql" driver.

from django.db.models import *

class A(Model):
    name = CharField(maxlength=200)

    class Admin: pass

class B(Model):
    name = CharField(maxlength=200)

    class Admin: pass

class C(Model):
    name = CharField(maxlength=200)
    a = ForeignKey(A)
    b = ForeignKey(B)

    class Meta:
        unique_together = (("a","b"),)

    class Admin: pass

in the admin:

  1. create an instance of A
  2. go to create an instance of C, and fill out the "name" field, and select an "a" object, but do not select a "b" object
  3. save the instance

you will get an exception saying:

ERROR: invalid input syntax for integer: 
"" SELECT "main_c"."id","main_c"."name","main_c"."a_id","main_c"."b_id" 
FROM "main_c" INNER JOIN "main_a" AS "main_c__a" ON "main_c"."a_id" = "main_c__a"."id" 
WHERE ("main_c"."b_id" = '' AND "main_c__a"."id" ILIKE '1')

(isn't that ILIKE '1' quite nice for a numeric-key? :))

btw. this works fine in sqlite3, probably because it simply accepts this syntax...

comment:20 by Gábor Farkas <gabor@…>, 17 years ago

the stacktrace:

Traceback (most recent call last):
File "/home/gabor/src/django/django/core/handlers/base.py" in get_response
  77. response = callback(request, *callback_args, **callback_kwargs)
File "/home/gabor/src/django/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/home/gabor/src/django/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/home/gabor/src/django/django/contrib/admin/views/main.py" in add_stage
  257. errors = manipulator.get_validation_errors(new_data)
File "/home/gabor/src/django/django/oldforms/__init__.py" in get_validation_errors
  61. errors.update(field.get_validation_errors(new_data))
File "/home/gabor/src/django/django/oldforms/__init__.py" in get_validation_errors
  378. self.run_validator(new_data, validator)
File "/home/gabor/src/django/django/oldforms/__init__.py" in run_validator
  368. validator(new_data.get(self.field_name, ''), new_data)
File "/home/gabor/src/django/django/utils/functional.py" in _curried
  3. return _curried_func(*(args+moreargs), **dict(kwargs, **morekwargs))
File "/home/gabor/src/django/django/db/models/manipulators.py" in manipulator_validator_unique_together
  303. old_obj = self.manager.get(**kwargs)
File "/home/gabor/src/django/django/db/models/manager.py" in get
  69. return self.get_query_set().get(*args, **kwargs)
File "/home/gabor/src/django/django/db/models/query.py" in get
  261. obj_list = list(clone)
File "/home/gabor/src/django/django/db/models/query.py" in __iter__
  114. return iter(self._get_data())
File "/home/gabor/src/django/django/db/models/query.py" in _get_data
  482. self._result_cache = list(self.iterator())
File "/home/gabor/src/django/django/db/models/query.py" in iterator
  189. cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params)
File "/home/gabor/src/django/django/db/backends/util.py" in execute
  19. return self.cursor.execute(sql, params)
File "/home/gabor/src/django/django/db/backends/postgresql/base.py" in execute
  47. return self.cursor.execute(smart_str(sql, self.charset), self.format_params(params))

  ProgrammingError at /admin/main/c/add/
  ERROR: invalid input syntax for integer: "" SELECT "main_c"."id","main_c"."name","main_c"."a_id","main_c"."b_id" FROM "main_c" INNER JOIN "main_a" AS "main_c__a" ON "main_c"."a_id" = "main_c__a"."id" WHERE ("main_c"."b_id" = '' AND "main_c__a"."id" ILIKE '2')

comment:21 by David Larlet, 16 years ago

Cc: larlet@… added

comment:22 by Simon Ditner <simon-django@…>, 16 years ago

I found that the above patches weren't effective for 0.96, and also did not handle BooleanField correctly with sqlite3, as the value is stored as True/False as opposed to 'on' -- so the generated <name>__iexact = 'on' would not match results correctly. I wrote the following patch for 0.96 that seems to resolve this issue:

Index: manipulators.py
===================================================================
--- manipulators.py
+++ manipulators.py
@@ -278,6 +278,7 @@

 def manipulator_validator_unique_together(field_name_list, opts, self, field_data, all_data):
     from django.db.models.fields.related import ManyToOneRel
+    from django.db.models.fields import BooleanField
     from django.utils.text import get_text_list
     field_list = [opts.get_field(field_name) for field_name in field_name_list]
     if isinstance(field_list[0].rel, ManyToOneRel):
@@ -294,7 +295,15 @@
             # doesn't have blank=True.
             return
         if isinstance(f.rel, ManyToOneRel):
-            kwargs['%s__pk' % f.name] = field_val
+            if field_val is '':
+                kwargs['%s__isnull' % f.name] = True
+            else:
+                kwargs['%s__pk' % f.name] = field_val
+        elif isinstance(f, BooleanField):
+            if field_val in ('on', 'True', 'true', True):
+               kwargs['%s' % f.name] = True
+            else:
+               kwargs['%s' % f.name] = False
         else:
             kwargs['%s__iexact' % f.name] = field_val
     try:

comment:23 by Gary Wilson, 16 years ago

milestone: 1.0

comment:24 by cmarshal, 16 years ago

Owner: changed from nobody to cmarshal
Status: newassigned

comment:25 by cmarshal, 16 years ago

I've tried the above scenarios and the only way I am currently able to produce an IntegrityError with unique_together FKs is by adding an actual duplicate -- but this is the issue in #8209.

I'm going to look into #8209 and then see if this is resolved..

comment:26 by cmarshal, 16 years ago

Owner: changed from cmarshal to nobody
Status: assignednew

I cannot reproduce this one so I'll release it. Perhaps someone else can resolve?

comment:27 by Gary Wilson, 16 years ago

Owner: changed from nobody to Gary Wilson
Status: newassigned

comment:28 by Jacob, 16 years ago

I suspect that this is no longer a problem, especially since manipulators are no longer used and about to be removed. Can anyone verify that this still is a problem?

comment:29 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: assignedclosed

Agreed. The impending removal of manipulators (part of #7742) would seem to make this fixed through attrition.

comment:30 by Jacob, 12 years ago

milestone: 1.0

Milestone 1.0 deleted

comment:31 by david.queija@…, 12 years ago

Easy pickings: unset
Resolution: fixed
Status: closedreopened
Triage Stage: AcceptedUnreviewed
UI/UX: unset
Version: 1.3

comment:32 by anonymous, 12 years ago

I am having the problem described in this ticket with following model

class Vehicle(models.Model):

    class Meta:
        verbose_name = _('Vehicle')
        verbose_name_plural = _('Vehicles')
        unique_together = (('vehicle_identifier', 'company'),)

    objects = VehicleManager()

    name = models.CharField(max_length=32, unique=True,
                            verbose_name=_('Vehicle name'), default="")
    description = models.TextField(max_length=300,
                                   verbose_name=_('Description'),
                                   default="")
    model = models.ForeignKey(CarModel, verbose_name=_('Car model'),
                              default="", null=True, blank=True)
    car_plate = models.CharField(max_length=7, unique=True,
                                 verbose_name=_('Car plate'), default="")

    vehicle_identifier = models.CharField(max_length=32,
                                          verbose_name=_('Vehicle Identifier'),
                                          default="")

    # purchase attributes
    fecha_compra = models.DateTimeField(null=True, blank=True)
    fecha_baja = models.DateTimeField(null=True, blank=True)
    valor_compra = models.DecimalField(max_digits=9, decimal_places=2,
                                       null=True, blank=True)

    #Extras
    has_automatic_transmission = models.BooleanField(verbose_name=_('Automatic transmission'))
    has_mp3 = models.BooleanField(verbose_name=_('MP3'))
    has_climate = models.BooleanField(verbose_name=_('Climate'))
    pets_allowed = models.BooleanField(verbose_name=_('Pets allowed'))
    has_gps = models.BooleanField(verbose_name=_('GPS'))
    luggage_capacity_gt_200 = models.BooleanField(verbose_name=_('Luggage capacity greater than 200L'))

    image = models.ImageField(verbose_name=_('Image'),
                              upload_to=get_image_path,
                              null=True, blank=True,)

    # This fields will be updated with data received in trip data convadis messages
    last_location_lat = models.DecimalField(verbose_name=_('Latitude'),
                    max_digits=17, decimal_places=15,
                    null=True, blank=True,
                )  # 2.15
    last_location_long = models.DecimalField(verbose_name=_('Longitude'),
                    max_digits=17, decimal_places=15,
                    null=True, blank=True,
                )  # 2.15

    # Relationships
    station = models.ForeignKey(Station, verbose_name=_('Station'),
                                null=True, blank=True)
    company = models.ForeignKey(Company, verbose_name=_('Company'),
                                null=True, blank=True)

    # Automatic saved fields
    created_by = models.ForeignKey(User, related_name='vehicle_created_by')
    created_on = models.DateTimeField(auto_now_add=True)
    edited_by = models.ForeignKey(User, related_name='vehicle_edited_by')
    edited_on = models.DateTimeField(auto_now=True)

    # TODO replace this with a better idea, this is temporally only for demo
    def image_url(self):
        import settings
        return settings.MEDIA_URL + str(self.image)

    def save(self, *args, **kwargs):
        if self.last_location_lat == 0 and self.station:
            self.last_location_lat = self.station.location_lat
        if self.last_location_long == 0 and self.station:
            self.last_location_long = self.station.location_long
        super(Vehicle, self).save(*args, **kwargs)

    def __unicode__(self):
        return (self.car_plate + ' - ' +
                unicode(self.name) + ' (' +
                unicode(self.model)) + ')'

comment:33 by Carl Meyer, 12 years ago

Resolution: fixed
Status: reopenedclosed
Triage Stage: UnreviewedAccepted

The problem this ticket identified definitely no longer exists in the Django codebase. Also, unique_together certainly does work with FK fields, I use it frequently. So whatever problem you are having with your model, it is something else. Please use user-support channels (#django on Freenode, the django-users mailing list), and only file a ticket if you are sure that the problem is in Django itself, and you can reduce it to a simplest-case reproduction.

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