Opened 12 years ago
Closed 12 years ago
#18896 closed New feature (fixed)
get_or_create breaks for ManyToMany
Reported by: | Matt Long | 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 |
Description
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 Tag.objects.create(text='foo') #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/related.py", 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/manager.py", 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/query.py", line 449, in get_or_create obj.save(force_insert=True, using=self.db) File "/usr/local/lib/python2.7/dist-packages/django/db/models/base.py", 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/base.py", 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/manager.py", line 203, in _insert return insert_query(self.model, objs, fields, **kwargs) File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", 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/compiler.py", line 910, in execute_sql cursor.execute(sql, params) File "/usr/local/lib/python2.7/dist-packages/django/db/backends/util.py", line 40, in execute return self.cursor.execute(sql, params) File "/usr/local/lib/python2.7/dist-packages/django/db/backends/sqlite3/base.py", 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:2 by , 12 years ago
Owner: | changed from | to
---|
comment:3 by , 12 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|---|
Type: | Bug → 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 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
Yeah, this is a bug; get_or_create
shouldn't work one place and not the other.
comment:5 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |