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

[alastair.porter@…]

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).

[Claude Paroz]

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

[loic84]

@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.

Edit: Probably shouldn't look at the ORM code at 4AM. The self.model(**params) part can/should be moved outside of the try/except construct, only the save logic needs to be there.

[loic84]

PR ​https://github.com/django/django/pull/1385

[Tim Graham <timograham@…>]

In e716518ad29898fb25c820023aaf2fdd1c9eb4af:

Fixed #20761 -- Fixed DatabaseError handling in get_or_create and update_or_create.

[Tim Graham <timograham@…>]

In ad98b985aa18fbca65b45aae2421b985904bf561:

Fixed test failures introduced in e716518ad29898fb25c820023aaf2fdd1c9eb4af

refs #20761

[Claude Paroz <claude@…>]

In 311c1d2848bde59bf03627e5c3d72b319285201b:

Fixed #20761 -- Reworded ForeignKey default error message

[Claude Paroz]

Oups, sorry for the wrong ticket number, the latest commit above refers to #20791