Skip to content

Commit

Permalink
Fixed #21171 -- Avoided starting a transaction when a single (or atom…
Browse files Browse the repository at this point in the history
…ic queries) are executed.

django#10448
  • Loading branch information
benjamin-lim committed Jun 17, 2021
1 parent 7fc8f65 commit caa05ce
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 21 deletions.
1 change: 1 addition & 0 deletions django/contrib/gis/db/backends/oracle/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.db.backends.oracle.base import (
DatabaseWrapper as OracleDatabaseWrapper,
)
from contextlib import contextmanager

from .features import DatabaseFeatures
from .introspection import OracleIntrospection
Expand Down
39 changes: 23 additions & 16 deletions django/db/backends/oracle/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import platform
import sys
import warnings
from contextlib import contextmanager

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
Expand Down Expand Up @@ -63,6 +64,23 @@ def _setup_environment(environ):
from .utils import Oracle_datetime # NOQA isort:skip


@contextmanager
def wrap_oracle_errors():
try:
yield
except Database.DatabaseError as e:
# cx_Oracle raises a cx_Oracle.DatabaseError exception with the
# following attributes and values:
# code = 2091
# message = 'ORA-02091: transaction rolled back
# 'ORA-02291: integrity constraint (TEST_DJANGOTEST.SYS
# _C00102056) violated - parent key not found'
# Convert that case to Django's IntegrityError exception.
x = e.args[0]
if hasattr(x, 'code') and hasattr(x, 'message') and x.code == 2091 and 'ORA-02291' in x.message:
raise utils.IntegrityError(*tuple(e.args))
raise

class _UninitializedOperatorsDescriptor(object):

def __get__(self, instance, cls=None):
Expand Down Expand Up @@ -256,21 +274,8 @@ def create_cursor(self, name=None):

def _commit(self):
if self.connection is not None:
try:
with wrap_oracle_errors():
return self.connection.commit()
except Database.DatabaseError as e:
# cx_Oracle raises a cx_Oracle.DatabaseError exception
# with the following attributes and values:
# code = 2091
# message = 'ORA-02091: transaction rolled back
# 'ORA-02291: integrity constraint (TEST_DJANGOTEST.SYS
# _C00102056) violated - parent key not found'
# We convert that particular case to our IntegrityError exception
x = e.args[0]
if hasattr(x, 'code') and hasattr(x, 'message') \
and x.code == 2091 and 'ORA-02291' in x.message:
six.reraise(utils.IntegrityError, utils.IntegrityError(*tuple(e.args)), sys.exc_info()[2])
raise

# Oracle doesn't support releasing savepoints. But we fake them when query
# logging is enabled to keep query counts consistent with other backends.
Expand Down Expand Up @@ -494,7 +499,8 @@ def _fix_for_params(self, query, params, unify_by_values=False):
def execute(self, query, params=None):
query, params = self._fix_for_params(query, params, unify_by_values=True)
self._guess_input_sizes([params])
return self.cursor.execute(query, self._param_generator(params))
with wrap_oracle_errors():
return self.cursor.execute(query, self._param_generator(params))

def executemany(self, query, params=None):
if not params:
Expand All @@ -507,7 +513,8 @@ def executemany(self, query, params=None):
# more than once, we can't make it lazy by using a generator
formatted = [firstparams] + [self._format_params(p) for p in params_iter]
self._guess_input_sizes(formatted)
return self.cursor.executemany(query, [self._param_generator(p) for p in formatted])
with wrap_oracle_errors():
return self.cursor.executemany(query, [self._param_generator(p) for p in formatted])

def fetchone(self):
row = self.cursor.fetchone()
Expand Down
8 changes: 7 additions & 1 deletion django/db/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,13 @@ def save_base(self, raw=False, force_insert=False,
sender=origin, instance=self, raw=raw, using=using,
update_fields=update_fields,
)
with transaction.atomic(using=using, savepoint=False):
# A transaction isn't needed if one query is issued.
if meta.parents:
context_manager = transaction.atomic(using=using, savepoint=False)
else:
context_manager = transaction.mark_for_rollback_on_error(using=using)

with context_manager:
if not raw:
self._save_parents(cls, using, update_fields)
updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
Expand Down
17 changes: 14 additions & 3 deletions django/db/models/deletion.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,12 @@ def can_fast_delete(self, objs, from_field=None):
"""
if from_field and from_field.remote_field.on_delete is not CASCADE:
return False
if not (hasattr(objs, 'model') and hasattr(objs, '_raw_delete')):
if hasattr(objs, '_meta'):
model = type(objs)
elif hasattr(objs, 'model') and hasattr(objs, '_raw_delete'):
model = objs.model
else:
return False
model = objs.model
if (signals.pre_delete.has_listeners(model) or
signals.post_delete.has_listeners(model) or
signals.m2m_changed.has_listeners(model)):
Expand All @@ -147,7 +150,7 @@ def can_fast_delete(self, objs, from_field=None):
for related in get_candidate_relations_to_delete(opts):
if related.field.remote_field.on_delete is not DO_NOTHING:
return False
for field in model._meta.private_fields:
for field in opts.private_fields:
if hasattr(field, 'bulk_related_objects'):
# It's something like generic foreign key.
return False
Expand Down Expand Up @@ -271,6 +274,14 @@ def delete(self):
# number of objects deleted for each model label
deleted_counter = Counter()

# Optimize for the case with a single obj and no dependencies
if len(self.data) == 1 and len(instances) == 1:
instance = list(instances)[0]
if self.can_fast_delete(instance):
with transaction.mark_for_rollback_on_error(using=self.using):
count = sql.DeleteQuery(model).delete_batch([instance.pk], self.using)
return count, {model._meta.label: count}

with transaction.atomic(using=self.using, savepoint=False):
# send pre_delete signals
for model, obj in self.instances_with_model():
Expand Down
2 changes: 1 addition & 1 deletion django/db/models/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ def update(self, **kwargs):
query.add_update_values(kwargs)
# Clear any annotations so that they won't be present in subqueries.
query._annotations = None
with transaction.atomic(using=self.db, savepoint=False):
with transaction.mark_for_rollback_on_error(using=self.db):
rows = query.get_compiler(self.db).execute_sql(CURSOR)
self._result_cache = None
return rows
Expand Down
27 changes: 27 additions & 0 deletions django/db/transaction.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from contextlib import contextmanager

from django.db import (
DEFAULT_DB_ALIAS, DatabaseError, Error, ProgrammingError, connections,
)
Expand Down Expand Up @@ -35,6 +37,31 @@ def set_autocommit(autocommit, using=None):
return get_connection(using).set_autocommit(autocommit)


@contextmanager
def mark_for_rollback_on_error(using=None):
"""
Internal low-level utility to mark a transaction as "needs rollback" when
an exception is raised while not enforcing the enclosed block to be in a
transaction. This is needed by Model.save() and friends to avoid starting a
transaction when in autocommit mode and a single query is executed.
It's equivalent to:
connection = get_connection(using)
if connection.get_autocommit():
yield
else:
with transaction.atomic(using=using, savepoint=False):
yield
but it uses low-level utilities to avoid performance overhead.
"""
try:
yield
except Exception:
connection = get_connection(using)
if connection.in_atomic_block:
connection.needs_rollback = True
raise


def commit(using=None):
"""
Commits a transaction.
Expand Down

0 comments on commit caa05ce

Please sign in to comment.