Opened 13 years ago

Closed 13 years ago

#16340 closed Cleanup/optimization (fixed)

get_or_create should preserve the original traceback.

Reported by: Dougal Matthews Owned by: Jonas Obrist
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

(more context in #15117)

get_or_create in django.db.models.query re-raises the last exception as below. This modifies the traceback and the exception should preserve this.

            except IntegrityError, e:
                transaction.savepoint_rollback(sid, using=self.db)
                try:
                    return self.get(**lookup), False
                except self.model.DoesNotExist:
                    raise e

this can be fixed with

                exc_info = sys.exc_info()
                try:
                    return self.get(**lookup), False
                except self.model.DoesNotExist:
                    raise exc_info[1], None, exc_info[2]

Attachments (4)

16340_get_or_create_traceback.patch (2.1 KB ) - added by Dougal Matthews 13 years ago.
16340_get_or_create_traceback.2.patch (2.1 KB ) - added by Dougal Matthews 13 years ago.
16340_get_or_create_traceback.3.patch (2.1 KB ) - added by Dougal Matthews 13 years ago.
16340.patch (2.3 KB ) - added by Jonas Obrist 13 years ago.
new patch (same as https://github.com/ojii/django/compare/django:master...ojii:16340)

Download all attachments as: .zip

Change History (9)

by Dougal Matthews, 13 years ago

by Dougal Matthews, 13 years ago

comment:1 by Dougal Matthews, 13 years ago

Has patch: set

Added a patch that preserves the original traceback with a test. The test isn't ideal since we need to check the contents of a traceback, but it looks for the save call since this is where the IntegrityError comes from (in other words its the safest thing we can bet on being there.)

Version 0, edited 13 years ago by Dougal Matthews (next)

comment:2 by Dougal Matthews, 13 years ago

For reference. This patch changes a traceback like this:

Traceback (most recent call last):
  File "/vagrant/tests/modeltests/get_or_create/tests.py", line 60, in test_get_or_create
    ManualPrimaryKeyTest.objects.get_or_create(id=1, data="Different")
  File "/vagrant/django/db/models/manager.py", line 135, in get_or_create
    return self.get_query_set().get_or_create(**kwargs)
  File "/vagrant/django/db/models/query.py", line 396, in get_or_create
    raise e
IntegrityError: PRIMARY KEY must be unique

to show the original full exception traceback:

Traceback (most recent call last):
  File "/vagrant/tests/modeltests/get_or_create/tests.py", line 60, in test_get_or_create
    ManualPrimaryKeyTest.objects.get_or_create(id=1, data="Different")
  File "/vagrant/django/db/models/manager.py", line 135, in get_or_create
    return self.get_query_set().get_or_create(**kwargs)
  File "/vagrant/django/db/models/query.py", line 386, in get_or_create
    obj.save(force_insert=True, using=self.db)
  File "/vagrant/django/db/models/base.py", line 463, in save
    self.save_base(using=using, force_insert=force_insert, force_update=force_update)
  File "/vagrant/django/db/models/base.py", line 556, in save_base
    result = manager._insert(values, return_id=update_pk, using=using)
  File "/vagrant/django/db/models/manager.py", line 198, in _insert
    return insert_query(self.model, values, **kwargs)
  File "/vagrant/django/db/models/query.py", line 1459, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/vagrant/django/db/models/sql/compiler.py", line 810, in execute_sql
    cursor = super(SQLInsertCompiler, self).execute_sql(None)
  File "/vagrant/django/db/models/sql/compiler.py", line 754, in execute_sql
    cursor.execute(sql, params)
  File "/vagrant/django/db/backends/sqlite3/base.py", line 226, in execute
    return Database.Cursor.execute(self, query, params)
IntegrityError: PRIMARY KEY must be unique

comment:3 by Ramiro Morales, 13 years ago

Triage Stage: UnreviewedAccepted

by Dougal Matthews, 13 years ago

comment:4 by Jonas Obrist, 13 years ago

Owner: changed from nobody to Jonas Obrist

Updated the patch to apply to trunk.

Also made use of the new unittest2 features (namely assertIn).

https://github.com/ojii/django/compare/django:master...ojii:16340

comment:5 by Julien Phalip, 13 years ago

Resolution: fixed
Status: newclosed

In [17333]:

Fixed #16340 -- Made get_or_create() re-raise any IntegrityError with its original traceback. Thanks to d0ugal and Jonas Obrist.

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