Check explicitly for mapped class as secondary

Added a comprehensive check and an informative error message for the case
where a mapped class, or a string mapped class name, is passed to
:paramref:`_orm.relationship.secondary`.  This is an extremely common error
which warrants a clear message.

Additionally, added a new rule to the class registry resolution such that
with regards to the :paramref:`_orm.relationship.secondary` parameter, if a
mapped class and its table are of the identical string name, the
:class:`.Table` will be favored when resolving this parameter.   In all
other cases, the class continues to be favored if a class and table
share the identical name.

Fixes: #5774
Change-Id: I65069d79c1c3897fbd1081a8e1edf3b63b497223

2 files changed, 96 insertions, 2 deletions

diff --git a/test/orm/declarative/test_basic.py b/test/orm/declarative/test_basic.py
index c21c63afc..fd00717f4 100644
--- a/test/orm/declarative/test_basic.py
+++ b/test/orm/declarative/test_basic.py
@@ -872,8 +872,8 @@ class DeclarativeTest(DeclarativeTestBase):
             props = relationship(
                 "Prop",
                 secondary="user_to_prop",
-                primaryjoin="User.id==user_to_prop.c.u" "ser_id",
-                secondaryjoin="user_to_prop.c.prop_id=" "=Prop.id",
+                primaryjoin="User.id==user_to_prop.c.user_id",
+                secondaryjoin="user_to_prop.c.prop_id==Prop.id",
                 backref="users",
             )
 
@@ -895,6 +895,59 @@ class DeclarativeTest(DeclarativeTestBase):
             class_mapper(User).get_property("props").secondary is user_to_prop
         )
 
+    def test_string_dependency_resolution_table_over_class(self):
+        # test for second half of #5774
+        class User(Base, fixtures.ComparableEntity):
+
+            __tablename__ = "users"
+            id = Column(Integer, primary_key=True)
+            name = Column(String(50))
+            props = relationship(
+                "Prop",
+                secondary="Secondary",
+                backref="users",
+            )
+
+        class Prop(Base, fixtures.ComparableEntity):
+
+            __tablename__ = "props"
+            id = Column(Integer, primary_key=True)
+            name = Column(String(50))
+
+        # class name and table name match
+        class Secondary(Base):
+            __tablename__ = "Secondary"
+            user_id = Column(Integer, ForeignKey("users.id"), primary_key=True)
+            prop_id = Column(Integer, ForeignKey("props.id"), primary_key=True)
+
+        configure_mappers()
+        assert (
+            class_mapper(User).get_property("props").secondary
+            is Secondary.__table__
+        )
+
+    def test_string_dependency_resolution_class_over_table(self):
+        # test for second half of #5774
+        class User(Base, fixtures.ComparableEntity):
+
+            __tablename__ = "users"
+            id = Column(Integer, primary_key=True)
+            name = Column(String(50))
+            secondary = relationship(
+                "Secondary",
+            )
+
+        # class name and table name match
+        class Secondary(Base):
+            __tablename__ = "Secondary"
+            user_id = Column(Integer, ForeignKey("users.id"), primary_key=True)
+
+        configure_mappers()
+        assert (
+            class_mapper(User).get_property("secondary").mapper
+            is Secondary.__mapper__
+        )
+
     def test_string_dependency_resolution_schemas(self):
         Base = declarative_base()
 
diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py
index 9a91197ed..22315e176 100644
--- a/test/orm/test_relationships.py
+++ b/test/orm/test_relationships.py
@@ -4355,6 +4355,47 @@ class AmbiguousFKResolutionTest(_RelationshipErrors, fixtures.MappedTest):
         sa.orm.configure_mappers()
 
 
+class SecondaryArgTest(fixtures.TestBase):
+    def teardown(self):
+        clear_mappers()
+
+    @testing.combinations((True,), (False,))
+    def test_informative_message_on_cls_as_secondary(self, string):
+        Base = declarative_base()
+
+        class C(Base):
+            __tablename__ = "c"
+            id = Column(Integer, primary_key=True)
+            a_id = Column(ForeignKey("a.id"))
+            b_id = Column(ForeignKey("b.id"))
+
+        if string:
+            c_arg = "C"
+        else:
+            c_arg = C
+
+        class A(Base):
+            __tablename__ = "a"
+
+            id = Column(Integer, primary_key=True)
+            data = Column(String)
+            bs = relationship("B", secondary=c_arg)
+
+        class B(Base):
+            __tablename__ = "b"
+            id = Column(Integer, primary_key=True)
+
+        assert_raises_message(
+            exc.ArgumentError,
+            r"secondary argument <class .*C.*> passed to to "
+            r"relationship\(\) A.bs "
+            "must be a Table object or other FROM clause; can't send a "
+            "mapped class directly as rows in 'secondary' are persisted "
+            "independently of a class that is mapped to that same table.",
+            configure_mappers,
+        )
+
+
 class SecondaryNestedJoinTest(
     fixtures.MappedTest, AssertsCompiledSQL, testing.AssertsExecutionResults
 ):