Opened 17 years ago

Closed 16 years ago

Last modified 13 years ago

#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


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/ in get_or_create(self, **kwargs)
     69     def get_or_create(self, **kwargs):
---> 70         return self.get_query_set().get_or_create(**kwargs)
     72     def create(self, **kwargs):

django/db/models/ in get_or_create(self, **kwargs)
    238             params.update(defaults)
    239             obj = self.model(**params)
--> 240   
    241             return obj, True

django/db/models/ 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/ 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/ 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)
     94     def executemany(self, query, param_list):

<class 'sqlite3.IntegrityError'>: testing_choice.poll_id may not be NULL

Attachments (1)

get_or_create.diff (3.7 KB ) - added by Jan Rademaker <j.rademaker@…> 17 years ago.
Patch + tests

Download all attachments as: .zip

Change History (12)

comment:1 by Antti Kaihola, 17 years ago

The work-around is:

php, created = what.choice_set.get_or_create(poll=what, choice='PHP', votes=1)

But it obviously isn't DRY :)

comment:2 by Brian Beck, 17 years ago

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 by Antti Kaihola, 17 years ago

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.

by Jan Rademaker <j.rademaker@…>, 17 years ago

Attachment: get_or_create.diff added

Patch + tests

comment:4 by Jan Rademaker <j.rademaker@…>, 17 years ago

Has patch: set

Basically, I just copied the get_or_create method from (QuerySet.get_or_create).

comment:5 by Jacob, 16 years ago

Triage Stage: UnreviewedAccepted

comment:6 by Christoph Neuroth <christoph.neuroth@…>, 16 years ago

Cc: christoph.neuroth@… added

comment:7 by Chris Beaven, 16 years ago

Triage Stage: AcceptedReady 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 by Malcolm Tredinnick, 16 years ago

milestone: 1.0
Triage Stage: Ready for checkinAccepted

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 by Gary Wilson, 16 years ago

Owner: changed from nobody to Gary Wilson
Status: newassigned

comment:10 by Gary Wilson, 16 years ago

Resolution: fixed
Status: assignedclosed

(In [8415]) Fixed #3121 -- Made get_or_create() work for RelatedManager and ManyRelatedManager.

comment:11 by Jacob, 13 years ago

milestone: 1.0

Milestone 1.0 deleted

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