Dont return outer transaction for _subtrans flag

Fixed critical regression where the :class:`_orm.Session` could fail to
"autobegin" a new transaction when a flush occurred without an existing
transaction in place, implicitly placing the :class:`_orm.Session` into
legacy autocommit mode which commit the transaction. The
:class:`_orm.Session` now has a check that will prevent this condition from
occurring, in addition to repairing the flush issue.

Additionally, scaled back part of the change made as part of :ticket:`5226`
which can run autoflush during an unexpire operation, to not actually
do this in the case of a :class:`_orm.Session` using legacy
:paramref:`_orm.Session.autocommit` mode, as this incurs a commit within
a refresh operation.

Fixes: #6233
Change-Id: Ia980e62a090e39e3e2a7fb77c95832ae784cc9a5

diff --git a/doc/build/changelog/unreleased_14/6233.rst b/doc/build/changelog/unreleased_14/6233.rst
new file mode 100644 (file)
index 0000000..80552e9
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6233.rst
@@ -0,0 +1,16 @@
+.. change::
+    :tags: bug, regression, orm
+    :tickets: 6233
+
+    Fixed critical regression where the :class:`_orm.Session` could fail to
+    "autobegin" a new transaction when a flush occurred without an existing
+    transaction in place, implicitly placing the :class:`_orm.Session` into
+    legacy autocommit mode which commit the transaction. The
+    :class:`_orm.Session` now has a check that will prevent this condition from
+    occurring, in addition to repairing the flush issue.
+
+    Additionally, scaled back part of the change made as part of :ticket:`5226`
+    which can run autoflush during an unexpire operation, to not actually
+    do this in the case of a :class:`_orm.Session` using legacy
+    :paramref:`_orm.Session.autocommit` mode, as this incurs a commit within
+    a refresh operation.
\ No newline at end of file

diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index 0dc92daf5bbe0941aa830e16dcd4173090d06ec5..12acfc5b786e891c1896ef95b71432c35e6cd30a 100644 (file)
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -1343,7 +1343,9 @@ def load_scalar_attributes(mapper, state, attribute_names, passive):
 
     result = False
 
-    no_autoflush = bool(passive & attributes.NO_AUTOFLUSH)
+    no_autoflush = (
+        bool(passive & attributes.NO_AUTOFLUSH) or state.session.autocommit
+    )
 
     # in the case of inheritance, particularly concrete and abstract
     # concrete inheritance, the class manager might have some keys

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 0562569bff23022cac58b79da9b29995fcf01475..d08abb78801713422086f4e7f8abcb1e625f4dc1 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1140,8 +1140,7 @@ class Session(_SessionClassMethods):
         if autocommit:
             if future:
                 raise sa_exc.ArgumentError(
-                    "Cannot use autocommit mode with future=True.  "
-                    "use the autobegin flag."
+                    "Cannot use autocommit mode with future=True."
                 )
             self.autocommit = True
         else:
@@ -1308,7 +1307,7 @@ class Session(_SessionClassMethods):
                 "Session objects."
             )
         if self._autobegin():
-            if not subtransactions and not nested:
+            if not subtransactions and not nested and not _subtrans:
                 return self._transaction
 
         if self._transaction is not None:
@@ -1321,9 +1320,18 @@ class Session(_SessionClassMethods):
                 raise sa_exc.InvalidRequestError(
                     "A transaction is already begun on this Session."
                 )
+        elif not self.autocommit:
+            # outermost transaction.  must be a not nested and not
+            # a subtransaction
+            assert not nested and not _subtrans and not subtransactions
+            trans = SessionTransaction(self)
+            assert self._transaction is trans
         else:
+            # legacy autocommit mode
+            assert not self.future
             trans = SessionTransaction(self, nested=nested)
             assert self._transaction is trans
+
         return self._transaction  # needed for __enter__/__exit__ hook
 
     def begin_nested(self):

diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index 280bb99b8bab494ca478d6fedc0dc6ce5baf7d34..89d61c3ea1b673cca4eb0b1ea9ababe1bef5a884 100644 (file)
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -151,6 +151,64 @@ class TransScopingTest(_fixtures.FixtureTest):
         s.commit()
         is_(s._transaction, None)
 
+    def test_autobegin_within_flush(self):
+        """test :ticket:`6233`"""
+
+        s = Session(testing.db)
+
+        User, users = self.classes.User, self.tables.users
+
+        mapper(User, users)
+        s.add(User(name="u1"))
+        s.commit()
+
+        u1 = s.query(User).first()
+
+        s.commit()
+
+        u1.name = "newname"
+
+        s.flush()
+
+        eq_(s.connection().scalar(select(User.name)), "newname")
+        assert s.in_transaction()
+        s.rollback()
+        assert not s.in_transaction()
+        eq_(s.connection().scalar(select(User.name)), "u1")
+
+    def test_no_autoflush_or_commit_in_expire_w_autocommit(self):
+        """test second part of :ticket:`6233`.
+
+        Here we test that the "autoflush on unexpire" feature added
+        in :ticket:`5226` is turned off for a legacy autocommit session.
+
+        """
+
+        s = Session(
+            testing.db, autocommit=True, expire_on_commit=True, autoflush=True
+        )
+
+        User, users = self.classes.User, self.tables.users
+
+        mapper(User, users)
+
+        u1 = User(name="u1")
+        s.add(u1)
+        s.flush()  # this commits
+
+        u1.name = "u2"  # this does not commit
+
+        assert "id" not in u1.__dict__
+        u1.id  # this unexpires
+
+        # never expired
+        eq_(u1.__dict__["name"], "u2")
+
+        eq_(u1.name, "u2")
+
+        # still in dirty collection
+        assert u1 in s.dirty
+
     def test_autobegin_begin_method(self):
         s = Session(testing.db)