Context Navigation

← Previous Ticket
Next Ticket →

#20761 closed Bug (fixed)

UnboundLocalError: local variable 'sid' referenced before assignment when using get_or_create with a badly formed init

Reported by:	alastair.porter@…	Owned by:	loic84
Component:	Database layer (models, ORM)	Version:	dev
Severity:	Normal	Keywords:
Cc:		Triage Stage:	Accepted
Has patch:	yes	Needs documentation:	no
Needs tests:	no	Patch needs improvement:	no
Easy pickings:	no	UI/UX:	no

Description

The cause of this bug seems to be a little convoluted. I don't expect it to happen in normal use, but it still seems to be unexpected behaviour.

The smallest testcase I can create it with is:

class Test(models.Model):
    def __init__(self, **kwargs):
        import django.db
        raise django.db.IntegrityError("oops")

The cause of the Integrity error isn't important. I was trying some stuff in a custom init that didn't end up working. The error I get is:

In [2]: models.Test.objects.get_or_create(id=1)

/env/local/lib/python2.7/site-packages/django/db/models/manager.pyc in get_or_create(self, **kwargs)
    144 
    145     def get_or_create(self, **kwargs):
--> 146         return self.get_query_set().get_or_create(**kwargs)
    147 
    148     def create(self, **kwargs):

/env/local/lib/python2.7/site-packages/django/db/models/query.pyc in get_or_create(self, **kwargs)
    479                 return obj, True
    480             except IntegrityError as e:
--> 481                 transaction.savepoint_rollback(sid, using=self.db)
    482                 exc_info = sys.exc_info()
    483                 try:

UnboundLocalError: local variable 'sid' referenced before assignment

The reason for this seems to be that the try: block in query.py:get_or_create expects the IntegrityError to be thrown when the new object is saved, not when it's created (line 476).

Change History (8)

comment:1 by Claude Paroz, 11 years ago

Component:	Uncategorized → Database layer (models, ORM)
Triage Stage:	Unreviewed → Accepted

I guess that the obj = self.model(**params) line (now in _create_object_from_params in master) should be outside the try/except construct.

comment:2 by loic84, 11 years ago

@claudep the try/except construct is there to catch a race condition.

[0e51d8eb66121b2558a38c9f4e366df781046873] changed the method to handle transaction better, however I don't like how any error would lead to a self.get().

I'd suggest the following snippet:

except DatabaseError as e:
    transaction.savepoint_rollback(sid, using=self.db)
    if isinstance(e, IntegrityError):
        return self.get(**lookup), False

Refs. #20463 #20429 #12579 #13906

Version 0, edited 11 years ago by loic84 (next)