Opened 3 years ago

Closed 2 years ago

#18896 closed New feature (fixed)

get_or_create breaks for ManyToMany

Reported by: mattlong Owned by: pyriku
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Using get_or_create through a ManyToMany field results in an integrity error if the object being queried for already exists but is not yet associated with the parent object:

class Tag(models.Model): 
    text = models.CharField(max_length=256, unique=True) 

class Thing(models.Model): 
    name = models.CharField(max_length=256) 
    tags = models.ManyToManyField(Tag)

#create and save a Tag

#create and save a Thing
a_thing = Thing.objects.create(name='a')

#get the previously created Tag and have it associated with a_thing
a_thing.tags.get_or_create(text='foo') #should get 

Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/fields/", line 616, in get_or_create
    super(ManyRelatedManager, self.db_manager(db)).get_or_create(**kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/", line 134, in get_or_create
    return self.get_query_set().get_or_create(**kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/", line 449, in get_or_create, using=self.db)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/", line 463, in save
    self.save_base(using=using, force_insert=force_insert, force_update=force_update)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/", line 551, in save_base
    result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/", line 203, in _insert
    return insert_query(self.model, objs, fields, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/", line 1576, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/usr/local/lib/python2.7/dist-packages/django/db/models/sql/", line 910, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python2.7/dist-packages/django/db/backends/", line 40, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python2.7/dist-packages/django/db/backends/sqlite3/", line 337, in execute
    return Database.Cursor.execute(self, query, params)
IntegrityError: column text is not unique

To summarize, if a Tag with text 'foo' exists but is not yet associated with a given Thing instance, using .tags.get_or_create(text='foo') raises an IntegrityError since it tries to re-create the same Tag.

I'm not familiar with the Django ORM source code, but I've traced the issue to ManyRelatedManager's get_query_set method always including its core_filters. This results in the "get" portion of get_or_create to only return a hit if the Tag exists and is already associated to the calling Thing instance. Given the nature of a many-to-many relationship, it should not be a requirement that a Tag already be linked to the calling Thing for get_or_create to find it; it should be enough that the Tag simply exists. In this case, I would expect .tags.get_or_create(...) to just add/save the association between Thing and Tag and return the existing Tag.

Change History (7)

comment:1 Changed 3 years ago by thikonom

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

The same error showed up on my Mysql db when i executed your example ( IntegrityError: (1062, "Duplicate entry 'foo' for key 'text'") ).

Last edited 3 years ago by thikonom (previous) (diff)

comment:2 Changed 3 years ago by thikonom

  • Owner changed from nobody to thikonom

comment:3 Changed 3 years ago by melinath

  • Triage Stage changed from Unreviewed to Design decision needed
  • Type changed from Bug to New feature

Marking this DDN. Although I see your point and the solution seems reasonable to me, this seems like it could be more of a new feature than a bug - but I'm not sure.

comment:4 Changed 3 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

Yeah, this is a bug; get_or_create shouldn't work one place and not the other.

comment:5 Changed 2 years ago by pyriku

  • Owner changed from thikonom to pyriku
  • Status changed from new to assigned

comment:7 Changed 2 years ago by Pablo Recio <pablo@…>

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

In 65f9e0affd8ca04e2c597c43c1547ef7c888ec2a:

Fixes #18896. Add tests verifying that you can get IntegrityErrors using get_or_create through relations like M2M, and it also adds a note into the documentation warning about it

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