Code

Opened 8 years ago

Closed 2 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: gwilson
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"),)

Attachments (0)

Change History (33)

comment:1 Changed 8 years ago by Olive

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

comment:2 Changed 8 years ago by Olive

  • Severity changed from major to blocker

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 Changed 8 years ago by adrian

  • priority changed from high to normal
  • Severity changed from blocker to normal

comment:4 Changed 8 years ago by anonymous

Yeah, what's the deal with this?

comment:5 Changed 8 years ago by anonymous

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

comment:6 Changed 8 years ago by anonymous

  • Summary changed from unique_together does not work when any of the fields contains a FK or has edit_inline set to True to unique_together does not work when any of the listed fields contains a FK

comment:7 Changed 8 years ago by Gary Wilson <gary.wilson@…>

  • 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 follow-up: Changed 8 years ago by favo@…

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 Changed 8 years ago by favo@…

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 Changed 7 years ago by anonymous

  • Cc cathy@… added

comment:11 Changed 7 years ago by Noah Slater

  • Cc nslater@… added

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

comment:12 Changed 7 years ago by Gary Wilson <gary.wilson@…>

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 Changed 7 years ago by anonymous

Same here, PostgreSQL.

comment:14 follow-up: Changed 7 years ago by Cathy Young

I was on MySQL when I hit this problem.

comment:15 in reply to: ↑ 14 Changed 7 years ago by anonymous

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 Changed 7 years ago by Simon G. <dev@…>

  • Keywords fk unique_together added
  • Triage Stage changed from Unreviewed to Accepted

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

#2019 marked as a duplicate of this.

comment:18 in reply to: ↑ 8 Changed 7 years ago by John Shaffer <jshaffer2112@…>

Replying to favo@exoweb.net:

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

This change was made in [4028].

comment:19 Changed 7 years ago by Gábor Farkas <gabor@…>

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 Changed 7 years ago by Gábor Farkas <gabor@…>

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 Changed 6 years ago by david

  • Cc larlet@… added

comment:22 Changed 6 years ago by Simon Ditner <simon-django@…>

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 Changed 6 years ago by gwilson

  • milestone set to 1.0

comment:24 Changed 6 years ago by cmarshal

  • Owner changed from nobody to cmarshal
  • Status changed from new to assigned

comment:25 Changed 6 years ago by cmarshal

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 Changed 6 years ago by cmarshal

  • Owner changed from cmarshal to nobody
  • Status changed from assigned to new

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

comment:27 Changed 6 years ago by gwilson

  • Owner changed from nobody to gwilson
  • Status changed from new to assigned

comment:28 Changed 6 years ago by jacob

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 Changed 6 years ago by mtredinnick

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

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

comment:30 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

comment:31 Changed 2 years ago by david.queija@…

  • Easy pickings unset
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Accepted to Unreviewed
  • UI/UX unset
  • Version set to 1.3

comment:32 Changed 2 years ago by anonymous

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 Changed 2 years ago by carljm

  • Resolution set to fixed
  • Status changed from reopened to closed
  • Triage Stage changed from Unreviewed to Accepted

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.

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.