#3121 closed defect (fixed)
RelatedManager.get_or_create() does not work
Reported by: | Antti Kaihola | Owned by: | Gary Wilson |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | normal | Keywords: | |
Cc: | christoph.neuroth@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Given the following models:
class Poll(models.Model): question = models.CharField(maxlength=200) pub_date = models.DateTimeField('date published') class Choice(models.Model): poll = models.ForeignKey(Poll) choice = models.CharField(maxlength=200) votes = models.IntegerField()
you can do:
>>> what, created = Poll.objects.get_or_create(question='What?', pub_date='2006-12-09') >>> django = what.choice_set.create(choice='Django', votes=100) >>> python, created = Choice.objects.get_or_create(poll=what, choice='Python', votes=200)
But for some reason this doesn't work:
>>> php, created = what.choice_set.get_or_create(choice='PHP', votes=1)
but gives the following error and traceback:
<class 'sqlite3.IntegrityError'> Traceback (most recent call last) django/db/models/manager.py in get_or_create(self, **kwargs) 68 69 def get_or_create(self, **kwargs): ---> 70 return self.get_query_set().get_or_create(**kwargs) 71 72 def create(self, **kwargs): django/db/models/query.py in get_or_create(self, **kwargs) 238 params.update(defaults) 239 obj = self.model(**params) --> 240 obj.save() 241 return obj, True 242 django/db/models/base.py in save(self) 202 cursor.execute("INSERT INTO %s (%s) VALUES (%s)" % \ 203 (backend.quote_name(self._meta.db_table), ','.join(field_names), --> 204 ','.join(placeholders)), db_values) 205 else: 206 # Create a new record with defaults for everything. django/db/backends/util.py in execute(self, sql, params) 10 start = time() 11 try: ---> 12 return self.cursor.execute(sql, params) 13 finally: 14 stop = time() django/db/backends/sqlite3/base.py in execute(self, query, params) 90 def execute(self, query, params=()): 91 query = self.convert_query(query, len(params)) ---> 92 return Database.Cursor.execute(self, query, params) 93 94 def executemany(self, query, param_list): <class 'sqlite3.IntegrityError'>: testing_choice.poll_id may not be NULL
Attachments (1)
Change History (12)
comment:1 Changed 16 years ago by
comment:2 Changed 16 years ago by
This bug also affects generic relation managers (see #2316}).
I tried to debug this, but it's pretty elusive. I think the solution might be to define get_or_create in the subclass anywhere create is also defined in the subclass. For instance, GenericRelatedObjectManager and ManyRelatedManager (among others, perhaps) both define a create method, but no get_or_create method. get_or_create is not defined to actually call its own class's create method (in QuerySet for instance, it duplicates create's code).
comment:3 Changed 16 years ago by
Even worse, it seems that for many-to-many relations, the get_or_create
method does create the related object but doesn't add a relation nor gives any error message.
For instance if you have model "Tag
" with a field "name
" and a model "BlogPost
" with "tags=ManyToManyField(Tag)
", then
myblogpost.tags.get_or_create(name='language_wars', defaults={'name':'language_wars'})
will create a Tag
object with the correct name, but myblogpost.tags.all()
is left unchanged.
In my opinion, the get_or_create method for both ForeignKeys and ManyToManyFields should be removed or throw a NotImplemented exception as long as it doesn't work correctly. The current functionality will confuse people.
comment:4 Changed 16 years ago by
Has patch: | set |
---|
Basically, I just copied the get_or_create method from query.py (QuerySet.get_or_create).
comment:5 Changed 15 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
comment:6 Changed 15 years ago by
Cc: | christoph.neuroth@… added |
---|
comment:7 Changed 15 years ago by
Triage Stage: | Accepted → Ready for checkin |
---|
I've been using this patch for a while now. I can't see a reason why it's not ready for checkin.
comment:8 Changed 15 years ago by
milestone: | → 1.0 |
---|---|
Triage Stage: | Ready for checkin → Accepted |
This doesn't have the same error checking as normal get_or_create()
(if the save()
fails). The patch needs more work and isn't ready for checkin yet. Also needs savepoint()
additions to handle transactional support properly on PostgreSQL (again, as in the standard method).
The behaviour's inconsistent, so this is 1.0 if a proper patch can be produced.
comment:9 Changed 15 years ago by
Owner: | changed from nobody to Gary Wilson |
---|---|
Status: | new → assigned |
comment:10 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
The work-around is:
But it obviously isn't DRY :)