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

1 files changed, 16 insertions, 2 deletions

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index 13611f2bb..31a3b9ec9 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -20,6 +20,7 @@ import re
 import weakref
 
 from . import attributes
+from .base import _is_mapped_class
 from .base import state_str
 from .interfaces import MANYTOMANY
 from .interfaces import MANYTOONE
@@ -2163,9 +2164,13 @@ class RelationshipProperty(StrategizedProperty):
 
             if isinstance(attr_value, util.string_types):
                 setattr(
-                    self, attr, self._clsregistry_resolve_arg(attr_value)()
+                    self,
+                    attr,
+                    self._clsregistry_resolve_arg(
+                        attr_value, favor_tables=attr == "secondary"
+                    )(),
                 )
-            elif callable(attr_value):
+            elif callable(attr_value) and not _is_mapped_class(attr_value):
                 setattr(self, attr, attr_value())
 
         # remove "annotations" which are present if mapped class
@@ -2183,6 +2188,15 @@ class RelationshipProperty(StrategizedProperty):
                     ),
                 )
 
+        if self.secondary is not None and _is_mapped_class(self.secondary):
+            raise sa_exc.ArgumentError(
+                "secondary argument %s passed to to relationship() %s 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." % (self.secondary, self)
+            )
+
         # ensure expressions in self.order_by, foreign_keys,
         # remote_side are all columns, not strings.
         if self.order_by is not False and self.order_by is not None: