Merge pull request #3 from sendbird/commit

Fixed #21171 -- Avoided starting a transaction when a single

django/contrib/gis/db/backends/oracle/base.py

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

django/db/backends/oracle/base.py

@@ -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
@@ -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):
@@ -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.
@@ -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:
@@ -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()

django/db/models/base.py

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

django/db/models/deletion.py

@@ -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)):
@@ -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
@@ -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():

django/db/models/query.py

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

django/db/transaction.py

@@ -1,3 +1,5 @@
+from contextlib import contextmanager
+
 from django.db import (
     DEFAULT_DB_ALIAS, DatabaseError, Error, ProgrammingError, connections,
 )
@@ -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.