Django

Code

Ticket #11156 (new)

Opened 10 months ago

Last modified 3 weeks ago

Unnecessary savepoints with Oracle

Reported by: Richard Davies <richard.davies@elastichosts.com> Assigned to: nobody
Milestone: 1.3 Component: Database layer (models, ORM)
Version: SVN Keywords: oracle savepoints
Cc: richard.davies@elastichosts.com, mboersma, ikelly Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 0

Description

Savepoints are implemented in the Postgresql and Oracle backends, and provide a useful features to users of both.

In addition, the framework itself wraps various calls in savepoints (e.g. inside django/db/models/query.py:get_or_create). This is to work around a Postgresql-specific oddity that the open transaction needs to be rolled back to a prior savepoint if it experiences a database exception.

This oddity is not present on Oracle, so the extra savepoints are not required. We probably need to split the current single backend flag uses_savepoints into two: can_savepoint and needs_savepoint_after_exception.

See http://groups.google.com/group/django-developers/browse_thread/thread/bca33ecf27ff5d63

Attachments

11156.diff (7.0 kB) - added by ikelly on 12/23/09 13:15:40.
12037.diff (13.1 kB) - added by Richard Davies <richard.davies@elastichosts.com> on 01/01/10 06:56:43.

Change History

05/20/09 10:57:28 changed by mboersma

  • cc changed from richard.davies@elastichosts.com to richard.davies@elastichosts.com, mboersma, ikelly.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

08/08/09 15:42:07 changed by Alex

  • stage changed from Unreviewed to Design decision needed.

10/24/09 04:54:50 changed by Richard Davies <richard.davies@elastichosts.com>

  • milestone set to 1.2.

Adding to the 1.2 milestone, since it was accepted into the medium priority list (ORM-11).

12/22/09 15:33:03 changed by ikelly

  • keywords set to oracle savepoints.

12/23/09 13:15:40 changed by ikelly

  • attachment 11156.diff added.

12/23/09 13:22:00 changed by ikelly

  • has_patch set to 1.
  • needs_tests set to 1.
  • stage changed from Design decision needed to Accepted.

I've uploaded a patch for this based on Richard's suggestion. Before I commit this, I'd like some feedback from somebody more familiar with the Postgresql isolation level code. Also need to figure out a reasonable test case.

01/01/10 06:56:43 changed by Richard Davies <richard.davies@elastichosts.com>

  • attachment 12037.diff added.

01/01/10 07:10:50 changed by Richard Davies <richard.davies@elastichosts.com>

Ian - thanks for kicking this off.

I've updated your patch with:

a) Disabling needs_savepoint_on_exception on PostgreSQL before 8.0 where we can't do it anyway

b) A more conservative approach to the PostgreSQL isolation code - we can't use savepoints as currently written when we're in autocommit mode anyway, so just turn them off

c) Using the new only_if_needed flag in various test cases

d) Documentation

I'm not sure what a test case would reasonably be - the desired effect of this patch is that the test suite still runs on both PostgreSQL and Oracle, but is more efficient on Oracle than before - so I suggest that you just double check that spurious savepoints are no longer being generated on Oracle and then commit without a test case.

My only design concern is that I don't think only_if_needed is a great name. I'd be tempted to go with if_needs_savepoint_on_exception, which is rather long but at least explanatory!

03/03/10 17:04:47 changed by ubernostrum

  • milestone changed from 1.2 to 1.3.

So long as this isn't actually a bug (e.g., we're not ending up with an exception), this needs to be punted off 1.2.

Add/Change #11156 (Unnecessary savepoints with Oracle)




Change Properties
Action