]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
warn for local-only column in remote side
authorMike Bayer <mike_mp@zzzcomputing.com>
Sun, 25 Sep 2022 18:56:22 +0000 (14:56 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sun, 25 Sep 2022 20:17:33 +0000 (16:17 -0400)
A warning is emitted in ORM configurations when an explicit
:func:`_orm.remote` annotation is applied to columns that are local to the
immediate mapped class, when the referenced class does not include any of
the same table columns. Ideally this would raise an error at some point as
it's not correct from a mapping point of view.

Fixes: #7094
Fixes: #8575
Change-Id: Ia31be24aebe143161e19dc311b52c08fd5014d33

doc/build/changelog/unreleased_14/7094.rst [new file with mode: 0644]
lib/sqlalchemy/orm/relationships.py
test/orm/test_cycles.py
test/orm/test_relationships.py

diff --git a/doc/build/changelog/unreleased_14/7094.rst b/doc/build/changelog/unreleased_14/7094.rst
new file mode 100644 (file)
index 0000000..b6fb30d
--- /dev/null
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 7094
+
+    A warning is emitted in ORM configurations when an explicit
+    :func:`_orm.remote` annotation is applied to columns that are local to the
+    immediate mapped class, when the referenced class does not include any of
+    the same table columns. Ideally this would raise an error at some point as
+    it's not correct from a mapping point of view.
index 45b9b9beaaee0c5ecfc16bee1094cfbda2ad2e8f..48d60647c0f69d71e4f63f1409b6790562ecd1e7 100644 (file)
@@ -2806,6 +2806,22 @@ class JoinCondition:
                 "condition that are on the remote side of "
                 "the relationship." % (self.prop,)
             )
+        else:
+
+            not_target = util.column_set(
+                self.parent_persist_selectable.c
+            ).difference(self.child_persist_selectable.c)
+
+            for _, rmt in self.local_remote_pairs:
+                if rmt in not_target:
+                    util.warn(
+                        "Expression %s is marked as 'remote', but these "
+                        "column(s) are local to the local side.  The "
+                        "remote() annotation is needed only for a "
+                        "self-referential relationship where both sides "
+                        "of the relationship refer to the same tables."
+                        % (rmt,)
+                    )
 
     def _check_foreign_cols(
         self, join_condition: ColumnElement[bool], primary: bool
index 15155293faa80f2b243d942bd107db5710bede44..a23d8b735afea06796ac46fa79f1cd5af1bfb779 100644 (file)
@@ -918,7 +918,6 @@ class OneToManyManyToOneTest(fixtures.MappedTest):
                 favorite=relationship(
                     Ball,
                     primaryjoin=person.c.favorite_ball_id == ball.c.id,
-                    remote_side=person.c.favorite_ball_id,
                     post_update=True,
                     _legacy_inactive_history_style=(
                         self._legacy_inactive_history_style
@@ -1036,7 +1035,6 @@ class OneToManyManyToOneTest(fixtures.MappedTest):
                 favorite=relationship(
                     Ball,
                     primaryjoin=person.c.favorite_ball_id == ball.c.id,
-                    remote_side=person.c.favorite_ball_id,
                     _legacy_inactive_history_style=(
                         self._legacy_inactive_history_style
                     ),
@@ -1096,7 +1094,6 @@ class OneToManyManyToOneTest(fixtures.MappedTest):
                 favorite=relationship(
                     Ball,
                     primaryjoin=person.c.favorite_ball_id == ball.c.id,
-                    remote_side=person.c.favorite_ball_id,
                     _legacy_inactive_history_style=(
                         self._legacy_inactive_history_style
                     ),
index fd4230e150c322020768bc9c81de8b94668c8627..8d27742c3325e7a09162e2c251477673ec84c7d9 100644 (file)
@@ -2589,6 +2589,55 @@ class JoinConditionErrorTest(fixtures.TestBase):
         registry.map_imperatively(C2, t3)
         assert C1.c2.property.primaryjoin.compare(t1.c.id == t3.c.t1id)
 
+    @testing.combinations(
+        "annotation", "local_remote", argnames="remote_anno_type"
+    )
+    @testing.combinations("orm_col", "core_col", argnames="use_col_from")
+    def test_no_remote_on_local_only_cols(
+        self, decl_base, remote_anno_type, use_col_from
+    ):
+        """test #7094.
+
+        a warning should be emitted for an inappropriate remote_side argument
+
+        """
+
+        class A(decl_base):
+            __tablename__ = "a"
+
+            id = Column(Integer, primary_key=True)
+            data = Column(String)
+
+            if remote_anno_type == "annotation":
+                if use_col_from == "core_col":
+                    bs = relationship(
+                        "B",
+                        primaryjoin=lambda: remote(A.__table__.c.id)
+                        == B.__table__.c.a_id,
+                    )
+                elif use_col_from == "orm_col":
+                    bs = relationship(
+                        "B", primaryjoin="remote(A.id) == B.a_id"
+                    )
+            elif remote_anno_type == "local_remote":
+                if use_col_from == "core_col":
+                    bs = relationship(
+                        "B", remote_side=lambda: A.__table__.c.id
+                    )
+                elif use_col_from == "orm_col":
+                    bs = relationship("B", remote_side="A.id")
+
+        class B(decl_base):
+            __tablename__ = "b"
+            id = Column(Integer, primary_key=True)
+            a_id = Column(ForeignKey("a.id"))
+
+        with expect_warnings(
+            r"Expression a.id is marked as 'remote', but these column\(s\) "
+            r"are local to the local side. "
+        ):
+            decl_base.registry.configure()
+
     def test_join_error_raised(self, registry):
         m = MetaData()
         t1 = Table("t1", m, Column("id", Integer, primary_key=True))