From caa05ce9bbf6f53e46848aaa319b8b743b9db367 Mon Sep 17 00:00:00 2001 From: benjamin-lim Date: Thu, 17 Jun 2021 12:23:05 +0900 Subject: [PATCH] Fixed #21171 -- Avoided starting a transaction when a single (or atomic queries) are executed. https://github.com/django/django/pull/10448 --- django/contrib/gis/db/backends/oracle/base.py | 1 + django/db/backends/oracle/base.py | 39 +++++++++++-------- django/db/models/base.py | 8 +++- django/db/models/deletion.py | 17 ++++++-- django/db/models/query.py | 2 +- django/db/transaction.py | 27 +++++++++++++ 6 files changed, 73 insertions(+), 21 deletions(-) diff --git a/django/contrib/gis/db/backends/oracle/base.py b/django/contrib/gis/db/backends/oracle/base.py index 0093ef83bba8..98847722fb1e 100644 --- a/django/contrib/gis/db/backends/oracle/base.py +++ b/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 diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py index 2de74da5f81e..b504da8dc73e 100644 --- a/django/db/backends/oracle/base.py +++ b/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() diff --git a/django/db/models/base.py b/django/db/models/base.py index cf8f44919c22..9906053f29f3 100644 --- a/django/db/models/base.py +++ b/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) diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 590d5a229816..3ebe2d91219a 100644 --- a/django/db/models/deletion.py +++ b/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(): diff --git a/django/db/models/query.py b/django/db/models/query.py index cbf35610db18..35d6a702f25d 100644 --- a/django/db/models/query.py +++ b/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 diff --git a/django/db/transaction.py b/django/db/transaction.py index 412513b4751d..24c92cd445a7 100644 --- a/django/db/transaction.py +++ b/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.