summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/build/changelog/migration_14.rst53
-rw-r--r--lib/sqlalchemy/orm/session.py148
-rw-r--r--test/aaa_profiling/test_memusage.py45
-rw-r--r--test/aaa_profiling/test_orm.py6
-rw-r--r--test/orm/test_events.py10
-rw-r--r--test/orm/test_session.py59
-rw-r--r--test/orm/test_transaction.py4
7 files changed, 262 insertions, 63 deletions
diff --git a/doc/build/changelog/migration_14.rst b/doc/build/changelog/migration_14.rst
index a0a1f1f74..3a57191e3 100644
--- a/doc/build/changelog/migration_14.rst
+++ b/doc/build/changelog/migration_14.rst
@@ -398,6 +398,59 @@ as was present previously.
Behavioral Changes - ORM
========================
+.. _change_5074:
+
+Session does not immediately create a new SessionTransaction object
+----------------------------------------------------------------------------
+
+The :class:`.Session` object's default behavior of ``autocommit=False``
+historically has meant that there is always a :class:`.SessionTransaction`
+object in play, associated with the :class:`.Session` via the
+:attr:`.Session.transaction` attribute. When the given
+:class:`.SessionTransaction` was complete, due to a commit, rollback, or close,
+it was immediately replaced with a new one. The :class:`.SessionTransaction`
+by itself does not imply the usage of any connection-oriented resources, so
+this long-standing behavior has a particular elegance to it in that the state
+of :attr:`.Session.transaction` is always predictable as non-None.
+
+However, as part of the initiative in :ticket:`5056` to greatly reduce
+reference cycles, this assumption means that calling upon
+:meth:`.Session.close` results in a :class:`.Session` object that still has
+reference cycles and is more expensive to clean up, not to mention that there
+is a small overhead in constructing the :class:`.SessionTransaction`
+object, which meant that there would be unnecessary overhead created
+for a :class:`.Session` that for example invoked :meth:`.Session.commit`
+and then :meth:`.Session.close`.
+
+As such, it was decided that :meth:`.Session.close` should leave the internal
+state of ``self.transaction``, now referred to internally as
+``self._transaction``, as None, and that a new :class:`.SessionTransaction`
+should only be created when needed. For consistency and code coverage, this
+behavior was also expanded to include all the points at which "autobegin" is
+expected, not just when :meth:`.Session.close` were called.
+
+In particular, this causes a behavioral change for applications which
+subscribe to the :meth:`.SessionEvents.after_transaction_create` event hook;
+previously, this event would be emitted when the :class:`.Session` were first
+constructed, as well as for most actions that closed the previous transaction
+and would emit :meth:`.SessionEvents.after_transaction_end`. The new behavior
+is that :meth:`.SessionEvents.after_transaction_create` is emitted on demand,
+when the :class:`.Session` has not yet created a new
+:class:`.SessionTransaction` object and mapped objects are associated with the
+:class:`.Session` through methods like :meth:`.Session.add` and
+:meth:`.Session.delete`, when the :attr:`.Session.transaction` attribute is
+called upon, when the :meth:`.Session.flush` method has tasks to complete, etc.
+
+Besides the change in when the :meth:`.SessionEvents.after_transaction_create`
+event is emitted, the change should have no other user-visible impact on the
+:class:`.Session` object's behavior; the :class:`.Session` will continue to have
+the behavior that it remains usable for new operations after :meth:`.Session.close`
+is called, and the sequencing of how the :class:`.Session` interacts with the
+:class:`.Engine` and the database itself should also remain unaffected, since
+these operations were already operating in an on-demand fashion.
+
+:ticket:`5074`
+
.. _change_1763:
Eager loaders emit during unexpire operations
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 3c58b4260..fe0640a47 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -123,14 +123,14 @@ class SessionTransaction(object):
**Life Cycle**
- A :class:`.SessionTransaction` is associated with a :class:`.Session`
- in its default mode of ``autocommit=False`` immediately, associated
- with no database connections. As the :class:`.Session` is called upon
- to emit SQL on behalf of various :class:`.Engine` or :class:`.Connection`
- objects, a corresponding :class:`.Connection` and associated
- :class:`.Transaction` is added to a collection within the
- :class:`.SessionTransaction` object, becoming one of the
- connection/transaction pairs maintained by the
+ A :class:`.SessionTransaction` is associated with a :class:`.Session` in
+ its default mode of ``autocommit=False`` whenever the "autobegin" process
+ takes place, associated with no database connections. As the
+ :class:`.Session` is called upon to emit SQL on behalf of various
+ :class:`.Engine` or :class:`.Connection` objects, a corresponding
+ :class:`.Connection` and associated :class:`.Transaction` is added to a
+ collection within the :class:`.SessionTransaction` object, becoming one of
+ the connection/transaction pairs maintained by the
:class:`.SessionTransaction`. The start of a :class:`.SessionTransaction`
can be tracked using the :meth:`.SessionEvents.after_transaction_create`
event.
@@ -140,13 +140,18 @@ class SessionTransaction(object):
:meth:`.Session.close` methods are called. At this point, the
:class:`.SessionTransaction` removes its association with its parent
:class:`.Session`. A :class:`.Session` that is in ``autocommit=False``
- mode will create a new :class:`.SessionTransaction` to replace it
- immediately, whereas a :class:`.Session` that's in ``autocommit=True``
- mode will remain without a :class:`.SessionTransaction` until the
- :meth:`.Session.begin` method is called. The end of a
+ mode will create a new :class:`.SessionTransaction` to replace it when the
+ next "autobegin" event occurs, whereas a :class:`.Session` that's in
+ ``autocommit=True`` mode will remain without a :class:`.SessionTransaction`
+ until the :meth:`.Session.begin` method is called. The end of a
:class:`.SessionTransaction` can be tracked using the
:meth:`.SessionEvents.after_transaction_end` event.
+ .. versionchanged:: 1.4 the :class:`.SessionTransaction` is not created
+ immediately within a :class:`.Session` when constructed or when the
+ previous transaction is removed, it instead is created when the
+ :class:`.Session` is next used.
+
**Nesting and Subtransactions**
Another detail of :class:`.SessionTransaction` behavior is that it is
@@ -217,7 +222,7 @@ class SessionTransaction(object):
_rollback_exception = None
- def __init__(self, session, parent=None, nested=False):
+ def __init__(self, session, parent=None, nested=False, autobegin=False):
self.session = session
self._connections = {}
self._parent = parent
@@ -230,7 +235,7 @@ class SessionTransaction(object):
)
if self.session._enable_transaction_accounting:
- self._take_snapshot()
+ self._take_snapshot(autobegin=autobegin)
self.session.dispatch.after_transaction_create(self.session, self)
@@ -334,7 +339,7 @@ class SessionTransaction(object):
return result
- def _take_snapshot(self):
+ def _take_snapshot(self, autobegin=False):
if not self._is_transaction_boundary:
self._new = self._parent._new
self._deleted = self._parent._deleted
@@ -342,7 +347,7 @@ class SessionTransaction(object):
self._key_switches = self._parent._key_switches
return
- if not self.session._flushing:
+ if not autobegin and not self.session._flushing:
self.session.flush()
self._new = weakref.WeakKeyDictionary()
@@ -577,7 +582,7 @@ class SessionTransaction(object):
return self._parent
def close(self, invalidate=False):
- self.session.transaction = self._parent
+ self.session._transaction = self._parent
if self._parent is None:
for connection, transaction, autoclose in set(
self._connections.values()
@@ -592,9 +597,6 @@ class SessionTransaction(object):
self._state = CLOSED
self.session.dispatch.after_transaction_end(self.session, self)
- if self._parent is None:
- if not self.session.autocommit:
- self.session.begin()
self.session = None
self._connections = None
@@ -603,7 +605,7 @@ class SessionTransaction(object):
def __exit__(self, type_, value, traceback):
self._assert_active(deactive_ok=True, prepared_ok=True)
- if self.session.transaction is None:
+ if self.session._transaction is None:
return
if type_ is None:
try:
@@ -832,7 +834,7 @@ class Session(_SessionClassMethods):
self.__binds = {}
self._flushing = False
self._warn_on_events = False
- self.transaction = None
+ self._transaction = None
self.hash_key = _new_sessionid()
self.autoflush = autoflush
self.autocommit = autocommit
@@ -849,14 +851,24 @@ class Session(_SessionClassMethods):
for key, bind in binds.items():
self._add_bind(key, bind)
- if not self.autocommit:
- self.begin()
_sessions[self.hash_key] = self
connection_callable = None
- transaction = None
- """The current active or inactive :class:`.SessionTransaction`."""
+ @property
+ def transaction(self):
+ """The current active or inactive :class:`.SessionTransaction`.
+
+ If this session is in "autobegin" mode and the transaction was not
+ begun, this accessor will implicitly begin the transaction.
+
+ .. versionchanged:: 1.4 the :attr:`.Session.transaction` attribute
+ is now a read-only descriptor that will automatically start a
+ transaction in "autobegin" mode if one is not present.
+
+ """
+ self._autobegin()
+ return self._transaction
@util.memoized_property
def info(self):
@@ -873,6 +885,13 @@ class Session(_SessionClassMethods):
"""
return {}
+ def _autobegin(self):
+ if not self.autocommit and self._transaction is None:
+ self._transaction = SessionTransaction(self, autobegin=True)
+ return True
+
+ return False
+
def begin(self, subtransactions=False, nested=False):
"""Begin a transaction on this :class:`.Session`.
@@ -925,17 +944,22 @@ class Session(_SessionClassMethods):
"""
- if self.transaction is not None:
+
+ if self._autobegin():
+ if not subtransactions and not nested:
+ return
+
+ if self._transaction is not None:
if subtransactions or nested:
- self.transaction = self.transaction._begin(nested=nested)
+ self._transaction = self._transaction._begin(nested=nested)
else:
raise sa_exc.InvalidRequestError(
"A transaction is already begun. Use "
"subtransactions=True to allow subtransactions."
)
else:
- self.transaction = SessionTransaction(self, nested=nested)
- return self.transaction # needed for __enter__/__exit__ hook
+ self._transaction = SessionTransaction(self, nested=nested)
+ return self._transaction # needed for __enter__/__exit__ hook
def begin_nested(self):
"""Begin a "nested" transaction on this Session, e.g. SAVEPOINT.
@@ -977,10 +1001,10 @@ class Session(_SessionClassMethods):
:ref:`session_rollback`
"""
- if self.transaction is None:
+ if self._transaction is None:
pass
else:
- self.transaction.rollback()
+ self._transaction.rollback()
def commit(self):
"""Flush pending changes and commit the current transaction.
@@ -1010,13 +1034,11 @@ class Session(_SessionClassMethods):
:ref:`session_committing`
"""
- if self.transaction is None:
- if not self.autocommit:
- self.begin()
- else:
+ if self._transaction is None:
+ if not self._autobegin():
raise sa_exc.InvalidRequestError("No transaction is begun.")
- self.transaction.commit()
+ self._transaction.commit()
def prepare(self):
"""Prepare the current transaction in progress for two phase commit.
@@ -1029,13 +1051,11 @@ class Session(_SessionClassMethods):
:exc:`~sqlalchemy.exc.InvalidRequestError` is raised.
"""
- if self.transaction is None:
- if not self.autocommit:
- self.begin()
- else:
+ if self._transaction is None:
+ if not self._autobegin():
raise sa_exc.InvalidRequestError("No transaction is begun.")
- self.transaction.prepare()
+ self._transaction.prepare()
def connection(
self,
@@ -1117,8 +1137,10 @@ class Session(_SessionClassMethods):
)
def _connection_for_bind(self, engine, execution_options=None, **kw):
- if self.transaction is not None:
- return self.transaction._connection_for_bind(
+ self._autobegin()
+
+ if self._transaction is not None:
+ return self._transaction._connection_for_bind(
engine, execution_options
)
else:
@@ -1272,8 +1294,13 @@ class Session(_SessionClassMethods):
This clears all items and ends any transaction in progress.
If this session were created with ``autocommit=False``, a new
- transaction is immediately begun. Note that this new transaction does
- not use any connection resources until they are first needed.
+ transaction will be begun when the :class:`.Session` is next asked
+ to procure a database connection.
+
+ .. versionchanged:: 1.4 The :meth:`.Session.close` method does not
+ immediately create a new :class:`.SessionTransaction` object;
+ instead, the new :class:`.SessionTransaction` is created only if
+ the :class:`.Session` is used again for a database operation.
"""
self._close_impl(invalidate=False)
@@ -1313,8 +1340,8 @@ class Session(_SessionClassMethods):
def _close_impl(self, invalidate):
self.expunge_all()
- if self.transaction is not None:
- for transaction in self.transaction._iterate_self_and_parents():
+ if self._transaction is not None:
+ for transaction in self._transaction._iterate_self_and_parents():
transaction.close(invalidate)
def expunge_all(self):
@@ -1864,10 +1891,10 @@ class Session(_SessionClassMethods):
elif self.identity_map.contains_state(state):
self.identity_map.safe_discard(state)
self._deleted.pop(state, None)
- elif self.transaction:
+ elif self._transaction:
# state is "detached" from being deleted, but still present
# in the transaction snapshot
- self.transaction._deleted.pop(state, None)
+ self._transaction._deleted.pop(state, None)
statelib.InstanceState._detach_states(
states, self, to_transient=to_transient
)
@@ -1913,11 +1940,11 @@ class Session(_SessionClassMethods):
# state has already replaced this one in the identity
# map (see test/orm/test_naturalpks.py ReversePKsTest)
self.identity_map.safe_discard(state)
- if state in self.transaction._key_switches:
- orig_key = self.transaction._key_switches[state][0]
+ if state in self._transaction._key_switches:
+ orig_key = self._transaction._key_switches[state][0]
else:
orig_key = state.key
- self.transaction._key_switches[state] = (
+ self._transaction._key_switches[state] = (
orig_key,
instance_key,
)
@@ -1955,18 +1982,18 @@ class Session(_SessionClassMethods):
self._new.pop(state)
def _register_altered(self, states):
- if self._enable_transaction_accounting and self.transaction:
+ if self._enable_transaction_accounting and self._transaction:
for state in states:
if state in self._new:
- self.transaction._new[state] = True
+ self._transaction._new[state] = True
else:
- self.transaction._dirty[state] = True
+ self._transaction._dirty[state] = True
def _remove_newly_deleted(self, states):
persistent_to_deleted = self.dispatch.persistent_to_deleted or None
for state in states:
- if self._enable_transaction_accounting and self.transaction:
- self.transaction._deleted[state] = True
+ if self._enable_transaction_accounting and self._transaction:
+ self._transaction._deleted[state] = True
if persistent_to_deleted is not None:
# get a strong reference before we pop out of
@@ -2427,6 +2454,8 @@ class Session(_SessionClassMethods):
self._after_attach(state, obj)
def _before_attach(self, state, obj):
+ self._autobegin()
+
if state.session_id == self.hash_key:
return False
@@ -3083,7 +3112,8 @@ class Session(_SessionClassMethods):
:meth:`.SessionEvents.after_rollback` and related events.
"""
- return self.transaction and self.transaction.is_active
+ self._autobegin()
+ return self._transaction and self._transaction.is_active
identity_map = None
"""A mapping of object identities to objects themselves.
diff --git a/test/aaa_profiling/test_memusage.py b/test/aaa_profiling/test_memusage.py
index 301724509..71d5481d3 100644
--- a/test/aaa_profiling/test_memusage.py
+++ b/test/aaa_profiling/test_memusage.py
@@ -1457,3 +1457,48 @@ class CycleTest(_fixtures.FixtureTest):
visit_binary_product(visit, expr)
go()
+
+ def test_session_transaction(self):
+ @assert_cycles()
+ def go():
+ s = Session(testing.db)
+ s.connection()
+ s.close()
+
+ go()
+
+ def test_session_commit_rollback(self):
+ # this is enabled by #5074
+ @assert_cycles()
+ def go():
+ s = Session(testing.db)
+ s.connection()
+ s.commit()
+
+ go()
+
+ @assert_cycles()
+ def go():
+ s = Session(testing.db)
+ s.connection()
+ s.rollback()
+
+ go()
+
+ def test_session_multi_transaction(self):
+ @assert_cycles()
+ def go():
+ s = Session(testing.db)
+ assert s._transaction is None
+
+ s.connection()
+
+ s.close()
+ assert s._transaction is None
+
+ s.connection()
+ assert s._transaction is not None
+
+ s.close()
+
+ go()
diff --git a/test/aaa_profiling/test_orm.py b/test/aaa_profiling/test_orm.py
index e2df85a33..58c7de7ed 100644
--- a/test/aaa_profiling/test_orm.py
+++ b/test/aaa_profiling/test_orm.py
@@ -89,6 +89,8 @@ class MergeTest(fixtures.MappedTest):
# down from 185 on this this is a small slice of a usually
# bigger operation so using a small variance
+ sess2.transaction # autobegin
+
@profiling.function_call_count(variance=0.10)
def go1():
return sess2.merge(p1, load=False)
@@ -97,6 +99,8 @@ class MergeTest(fixtures.MappedTest):
# third call, merge object already present. almost no calls.
+ sess2.transaction # autobegin
+
@profiling.function_call_count(variance=0.10)
def go2():
return sess2.merge(p2, load=False)
@@ -115,6 +119,8 @@ class MergeTest(fixtures.MappedTest):
# using sqlite3 the C extension took it back up to approx. 1257
# (py2.6)
+ sess2.transaction # autobegin
+
@profiling.function_call_count(variance=0.10)
def go():
sess2.merge(p1)
diff --git a/test/orm/test_events.py b/test/orm/test_events.py
index b0b140ffa..6957a9030 100644
--- a/test/orm/test_events.py
+++ b/test/orm/test_events.py
@@ -1500,6 +1500,7 @@ class SessionEventsTest(_RemoveListeners, _fixtures.FixtureTest):
eq_(
canary,
[
+ "after_transaction_create", # changed in #5074
"before_attach",
"after_attach",
"before_commit",
@@ -1522,7 +1523,6 @@ class SessionEventsTest(_RemoveListeners, _fixtures.FixtureTest):
"after_transaction_end",
"after_soft_rollback",
"after_transaction_end",
- "after_transaction_create",
"after_soft_rollback",
],
)
@@ -1564,6 +1564,7 @@ class SessionEventsTest(_RemoveListeners, _fixtures.FixtureTest):
eq_(
canary,
[
+ "after_transaction_create", # changed due to #5074
"before_attach",
"after_attach",
"before_flush",
@@ -1599,7 +1600,7 @@ class SessionEventsTest(_RemoveListeners, _fixtures.FixtureTest):
"after_transaction_end",
"after_commit",
"after_transaction_end",
- "after_transaction_create",
+ # no longer autocreates after #5074
],
)
@@ -1647,10 +1648,10 @@ class SessionEventsTest(_RemoveListeners, _fixtures.FixtureTest):
eq_(
canary,
[
+ "after_transaction_create", # moved to top due to #5074
"before_commit",
"after_commit",
"after_transaction_end",
- "after_transaction_create",
],
)
@@ -1713,7 +1714,8 @@ class SessionEventsTest(_RemoveListeners, _fixtures.FixtureTest):
def test_connection_emits_after_begin(self):
sess, canary = self._listener_fixture(bind=testing.db)
sess.connection()
- eq_(canary, ["after_begin"])
+ # changed due to #5074
+ eq_(canary, ["after_transaction_create", "after_begin"])
sess.close()
def test_reentrant_flush(self):
diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index 4bc8e7691..a2f0a9aa1 100644
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -3,6 +3,7 @@ from sqlalchemy import event
from sqlalchemy import ForeignKey
from sqlalchemy import inspect
from sqlalchemy import Integer
+from sqlalchemy import select
from sqlalchemy import Sequence
from sqlalchemy import String
from sqlalchemy import testing
@@ -28,6 +29,7 @@ from sqlalchemy.testing import engines
from sqlalchemy.testing import eq_
from sqlalchemy.testing import fixtures
from sqlalchemy.testing import is_
+from sqlalchemy.testing import is_not_
from sqlalchemy.testing import is_true
from sqlalchemy.testing import mock
from sqlalchemy.testing import pickleable
@@ -121,6 +123,62 @@ class TransScopingTest(_fixtures.FixtureTest):
s.close()
c.execute("select * from users")
+ def test_autobegin_execute(self):
+ # test the new autobegin behavior introduced in #5074
+ s = Session(testing.db)
+
+ is_(s._transaction, None)
+
+ s.execute(select([1]))
+ is_not_(s._transaction, None)
+
+ s.commit()
+ is_(s._transaction, None)
+
+ s.execute(select([1]))
+ is_not_(s._transaction, None)
+
+ s.close()
+ is_(s._transaction, None)
+
+ s.execute(select([1]))
+ is_not_(s._transaction, None)
+
+ s.close()
+ is_(s._transaction, None)
+
+ def test_autobegin_flush(self):
+ # test the new autobegin behavior introduced in #5074
+ User, users = self.classes.User, self.tables.users
+
+ mapper(User, users)
+
+ s = Session(testing.db)
+
+ is_(s._transaction, None)
+
+ # empty flush, nothing happens
+ s.flush()
+ is_(s._transaction, None)
+
+ s.add(User(id=1, name="name"))
+ s.flush()
+ is_not_(s._transaction, None)
+ s.commit()
+ is_(s._transaction, None)
+
+ def test_autobegin_begin_method(self):
+ s = Session(testing.db)
+
+ s.begin() # OK
+
+ assert_raises_message(
+ sa.exc.InvalidRequestError,
+ "A transaction is already begun. Use "
+ "subtransactions=True to allow subtransactions.",
+ s.begin,
+ )
+
@testing.requires.independent_connections
@engines.close_open_connections
def test_transaction(self):
@@ -406,6 +464,7 @@ class SessionStateTest(_fixtures.FixtureTest):
sess.flush()
assert u1 not in sess
assert_raises(sa.exc.InvalidRequestError, sess.add, u1)
+ assert sess.transaction is not None
sess.rollback()
assert u1 in sess
diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py
index 3ad8880de..676a37e8d 100644
--- a/test/orm/test_transaction.py
+++ b/test/orm/test_transaction.py
@@ -550,6 +550,8 @@ class SessionTransactionTest(fixtures.RemovesEvents, FixtureTest):
def test_no_sql_during_rollback(self):
sess = create_session(bind=testing.db, autocommit=False)
+ sess.connection()
+
@event.listens_for(sess, "after_rollback")
def go(session):
session.execute("select 1")
@@ -1273,6 +1275,7 @@ class RollbackRecoverTest(_LocalFixture):
def test_pk_violation(self):
User, Address = self.classes.User, self.classes.Address
s = self.session()
+
a1 = Address(email_address="foo")
u1 = User(id=1, name="ed", addresses=[a1])
s.add(u1)
@@ -1287,6 +1290,7 @@ class RollbackRecoverTest(_LocalFixture):
with expect_warnings("New instance"):
assert_raises(sa_exc.IntegrityError, s.commit)
+
assert_raises(sa_exc.InvalidRequestError, s.commit)
s.rollback()
assert u2 not in s
View Lorry Controller Status