Continue to assume None for un-accessed attribute on persistent

object during m2o fetch

Fixed regression in new relationship m2o comparison logic first introduced
at :ref:`change_4359` when comparing to an attribute that is persisted as
NULL and is in an un-fetched state in the mapped instance.  Since the
attribute has no explicit default, it needs to default to NULL when
accessed in a persistent setting.

Fixes: #4676
Change-Id: I17160c30131187c735f025a785ff0276a246f6bb
(cherry picked from commit b6619f267ba6a327d6bff1aa650dd3ab9b718b29)

diff --git a/doc/build/changelog/unreleased_13/4676.rst b/doc/build/changelog/unreleased_13/4676.rst
new file mode 100644 (file)
index 0000000..aa814be
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4676.rst
@@ -0,0 +1,10 @@
+.. change::
+   :tags: bug, orm
+    :tickets: 4676
+
+    Fixed regression in new relationship m2o comparison logic first introduced
+    at :ref:`change_4359` when comparing to an attribute that is persisted as
+    NULL and is in an un-fetched state in the mapped instance.  Since the
+    attribute has no explicit default, it needs to default to NULL when
+    accessed in a persistent setting.
+

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index 008cfcd2a8c873a21e2aee69d6d09213adf2b465..b40fad332bfa418944f3c87a97f93c7e776df88b 100644 (file)
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -1574,7 +1574,7 @@ class RelationshipProperty(StrategizedProperty):
                 state,
                 dict_,
                 column,
-                passive=attributes.PASSIVE_RETURN_NEVER_SET
+                passive=attributes.PASSIVE_OFF
                 if state.persistent
                 else attributes.PASSIVE_NO_FETCH ^ attributes.INIT_OK,
             )

diff --git a/test/orm/_fixtures.py b/test/orm/_fixtures.py
index bb51777a31e3835e9fca5bb5667a858e2fdd7974..2b4fca148932b02c789c7347678b59a6da1c80d1 100644 (file)
--- a/test/orm/_fixtures.py
+++ b/test/orm/_fixtures.py
@@ -48,6 +48,9 @@ class FixtureTest(fixtures.MappedTest):
         class Dingaling(Base):
             pass
 
+        class HasDingaling(Base):
+            pass
+
         class Node(Base):
             pass
 
@@ -223,6 +226,17 @@ class FixtureTest(fixtures.MappedTest):
             test_needs_fk=True,
         )
 
+        Table(
+            "has_dingaling",
+            metadata,
+            Column(
+                "id", Integer, primary_key=True, test_needs_autoincrement=True
+            ),
+            Column("dingaling_id", None, ForeignKey("dingalings.id")),
+            test_needs_acid=True,
+            test_needs_fk=True,
+        )
+
         Table(
             "items",
             metadata,

diff --git a/test/orm/test_naturalpks.py b/test/orm/test_naturalpks.py
index 52c27f3158141a95a7652fe876700b0934348f02..982f342417d2ff8de25dc220280081617db99640 100644 (file)
--- a/test/orm/test_naturalpks.py
+++ b/test/orm/test_naturalpks.py
@@ -16,6 +16,7 @@ from sqlalchemy.orm.session import make_transient
 from sqlalchemy.testing import assert_raises
 from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import eq_
+from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import ne_
 from sqlalchemy.testing.schema import Column
@@ -677,6 +678,17 @@ class NaturalPKTest(fixtures.MappedTest):
 
         del u2.username
 
+        # object is persistent, so since we deleted, we get None
+        with expect_warnings("Got None for value of column "):
+            eq_(expr.left.callable(), None)
+
+        s.expunge(u2)
+
+        # however that None isn't in the dict, that's just the default
+        # attribute value, so after expunge it's gone
+        assert "username" not in u2.__dict__
+
+        # detached, we don't have it
         assert_raises_message(
             sa.exc.InvalidRequestError,
             "Can't resolve value for column users.username",

diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index b31647a7251e68b5f28a71dfa9abd0d383f47bd5..86b304be9de7b70caaf2182d52470dbc7b66ce78 100644 (file)
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -4604,8 +4604,18 @@ class WithTransientOnNone(_fixtures.FixtureTest, AssertsCompiledSQL):
     __dialect__ = "default"
 
     def _fixture1(self):
-        User, Address = self.classes.User, self.classes.Address
-        users, addresses = self.tables.users, self.tables.addresses
+        User, Address, Dingaling, HasDingaling = (
+            self.classes.User,
+            self.classes.Address,
+            self.classes.Dingaling,
+            self.classes.HasDingaling,
+        )
+        users, addresses, dingalings, has_dingaling = (
+            self.tables.users,
+            self.tables.addresses,
+            self.tables.dingalings,
+            self.tables.has_dingaling,
+        )
 
         mapper(User, users)
         mapper(
@@ -4622,6 +4632,20 @@ class WithTransientOnNone(_fixtures.FixtureTest, AssertsCompiledSQL):
                 ),
             },
         )
+        mapper(Dingaling, dingalings)
+        mapper(
+            HasDingaling,
+            has_dingaling,
+            properties={
+                "dingaling": relationship(
+                    Dingaling,
+                    primaryjoin=and_(
+                        dingalings.c.id == has_dingaling.c.dingaling_id,
+                        dingalings.c.data == "hi",
+                    ),
+                )
+            },
+        )
 
     def test_filter_with_transient_dont_assume_pk(self):
         self._fixture1()
@@ -4696,6 +4720,86 @@ class WithTransientOnNone(_fixtures.FixtureTest, AssertsCompiledSQL):
                 checkparams={"param_1": None, "param_2": None},
             )
 
+    def test_filter_with_persistent_non_pk_col_is_default_null(self):
+        # test #4676 - comparison to a persistent column that is
+        # NULL in the database, but is not fetched
+        self._fixture1()
+        Dingaling, HasDingaling = (
+            self.classes.Dingaling,
+            self.classes.HasDingaling,
+        )
+        s = Session()
+        d = Dingaling(id=1)
+        s.add(d)
+        s.flush()
+        assert "data" not in d.__dict__
+
+        q = s.query(HasDingaling).filter_by(dingaling=d)
+
+        with expect_warnings("Got None for value of column"):
+            self.assert_compile(
+                q,
+                "SELECT has_dingaling.id AS has_dingaling_id, "
+                "has_dingaling.dingaling_id AS has_dingaling_dingaling_id "
+                "FROM has_dingaling WHERE :param_1 = "
+                "has_dingaling.dingaling_id AND :param_2 = :data_1",
+                checkparams={"param_1": 1, "param_2": None, "data_1": "hi"},
+            )
+
+    def test_filter_with_detached_non_pk_col_is_default_null(self):
+        self._fixture1()
+        Dingaling, HasDingaling = (
+            self.classes.Dingaling,
+            self.classes.HasDingaling,
+        )
+        s = Session()
+        d = Dingaling()
+        s.add(d)
+        s.flush()
+        s.commit()
+        d.id
+        s.expire(d, ["data"])
+        s.expunge(d)
+        assert "data" not in d.__dict__
+        assert "id" in d.__dict__
+
+        q = s.query(HasDingaling).filter_by(dingaling=d)
+
+        # this case we still can't handle, object is detached so we assume
+        # nothing
+
+        assert_raises_message(
+            sa_exc.StatementError,
+            r"Can't resolve value for column dingalings.data on "
+            r"object .*Dingaling.* the object is detached and "
+            r"the value was expired",
+            q.all,
+        )
+
+    def test_filter_with_detached_non_pk_col_has_value(self):
+        self._fixture1()
+        Dingaling, HasDingaling = (
+            self.classes.Dingaling,
+            self.classes.HasDingaling,
+        )
+        s = Session()
+        d = Dingaling(data="some data")
+        s.add(d)
+        s.commit()
+        s.expire(d)
+        assert "data" not in d.__dict__
+
+        q = s.query(HasDingaling).filter_by(dingaling=d)
+
+        self.assert_compile(
+            q,
+            "SELECT has_dingaling.id AS has_dingaling_id, "
+            "has_dingaling.dingaling_id AS has_dingaling_dingaling_id "
+            "FROM has_dingaling WHERE :param_1 = "
+            "has_dingaling.dingaling_id AND :param_2 = :data_1",
+            checkparams={"param_1": 1, "param_2": "some data", "data_1": "hi"},
+        )
+
     def test_with_parent_with_transient_assume_pk(self):
         self._fixture1()
         User, Address = self.classes.User, self.classes.Address