- Improved the check for "how to join from A to B" such that when

a table has multiple, composite foreign keys targeting a parent table,
the :paramref:`.relationship.foreign_keys` argument will be properly
interpreted in order to resolve the ambiguity; previously this condition
would raise that there were multiple FK paths when in fact the
foreign_keys argument should be establishing which one is expected.
fixes #2965

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index f16dd4f89ad7f2d82db18f044ffd92b803543aad..f2594b7333b3f10bd7956b227135813382fa2b88 100644 (file)
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -14,6 +14,17 @@
 .. changelog::
     :version: 0.9.4
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 2965
+
+        Improved the check for "how to join from A to B" such that when
+        a table has multiple, composite foreign keys targeting a parent table,
+        the :paramref:`.relationship.foreign_keys` argument will be properly
+        interpreted in order to resolve the ambiguity; previously this condition
+        would raise that there were multiple FK paths when in fact the
+        foreign_keys argument should be establishing which one is expected.
+
     .. change::
         :tags: bug, mysql
 

diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index d59b45fae53623004e4b28d9b51b2336d5415cb8..7a220f949225e9b27270d3b9e0bf718d80778a26 100644 (file)
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -24,6 +24,7 @@ from .. import exc
 from operator import attrgetter
 from . import operators
 import operator
+import collections
 from .annotation import Annotated
 import itertools
 
@@ -682,8 +683,7 @@ class Join(FromClause):
         providing a "natural join".
 
         """
-        crit = []
-        constraints = set()
+        constraints = collections.defaultdict(list)
 
         for left in (a_subset, a):
             if left is None:
@@ -703,8 +703,7 @@ class Join(FromClause):
                         continue
 
                 if col is not None:
-                    crit.append(col == fk.parent)
-                    constraints.add(fk.constraint)
+                    constraints[fk.constraint].append((col, fk.parent))
             if left is not b:
                 for fk in sorted(
                             left.foreign_keys,
@@ -723,12 +722,36 @@ class Join(FromClause):
                             continue
 
                     if col is not None:
-                        crit.append(col == fk.parent)
-                        constraints.add(fk.constraint)
-            if crit:
+                        constraints[fk.constraint].append((col, fk.parent))
+            if constraints:
                 break
 
-        if len(crit) == 0:
+        if len(constraints) > 1:
+            # more than one constraint matched.  narrow down the list
+            # to include just those FKCs that match exactly to
+            # "consider_as_foreign_keys".
+            if consider_as_foreign_keys:
+                for const in list(constraints):
+                    if set(f.parent for f in const.elements) != set(consider_as_foreign_keys):
+                        del constraints[const]
+
+            # if still multiple constraints, but
+            # they all refer to the exact same end result, use it.
+            if len(constraints) > 1:
+                dedupe = set(tuple(crit) for crit in constraints.values())
+                if len(dedupe) == 1:
+                    key = constraints.keys()[0]
+                    constraints = {key, constraints[key]}
+
+            if len(constraints) != 1:
+                raise exc.AmbiguousForeignKeysError(
+                    "Can't determine join between '%s' and '%s'; "
+                    "tables have more than one foreign key "
+                    "constraint relationship between them. "
+                    "Please specify the 'onclause' of this "
+                    "join explicitly." % (a.description, b.description))
+
+        if len(constraints) == 0:
             if isinstance(b, FromGrouping):
                 hint = " Perhaps you meant to convert the right side to a "\
                                     "subquery using alias()?"
@@ -737,14 +760,9 @@ class Join(FromClause):
             raise exc.NoForeignKeysError(
                 "Can't find any foreign key relationships "
                 "between '%s' and '%s'.%s" % (a.description, b.description, hint))
-        elif len(constraints) > 1:
-            raise exc.AmbiguousForeignKeysError(
-                "Can't determine join between '%s' and '%s'; "
-                "tables have more than one foreign key "
-                "constraint relationship between them. "
-                "Please specify the 'onclause' of this "
-                "join explicitly." % (a.description, b.description))
-        elif len(crit) == 1:
+
+        crit = [(x == y) for x, y in constraints.values()[0]]
+        if len(crit) == 1:
             return (crit[0])
         else:
             return and_(*crit)

diff --git a/test/orm/test_rel_fn.py b/test/orm/test_rel_fn.py
index 10ba41429da1c10841295e6a7ae438632891eb90..0b35a44eb3d48f5f8bfb8ccdcc52c454a3145afd 100644 (file)
--- a/test/orm/test_rel_fn.py
+++ b/test/orm/test_rel_fn.py
@@ -104,6 +104,21 @@ class _JoinFixtures(object):
             Column('bid', Integer, ForeignKey('three_tab_b.id'))
         )
 
+        cls.composite_target = Table('composite_target', m,
+            Column('uid', Integer, primary_key=True),
+            Column('oid', Integer, primary_key=True),
+        )
+
+        cls.composite_multi_ref = Table('composite_multi_ref', m,
+            Column('uid1', Integer),
+            Column('uid2', Integer),
+            Column('oid', Integer),
+            ForeignKeyConstraint(("uid1", "oid"),
+                        ("composite_target.uid", "composite_target.oid")),
+            ForeignKeyConstraint(("uid2", "oid"),
+                        ("composite_target.uid", "composite_target.oid")),
+            )
+
     def _join_fixture_overlapping_three_tables(self, **kw):
         def _can_sync(*cols):
             for c in cols:
@@ -390,6 +405,17 @@ class _JoinFixtures(object):
                     **kw
                 )
 
+    def _join_fixture_overlapping_composite_fks(self, **kw):
+        return relationships.JoinCondition(
+                    self.composite_target,
+                    self.composite_multi_ref,
+                    self.composite_target,
+                    self.composite_multi_ref,
+                    consider_as_foreign_keys=[self.composite_multi_ref.c.uid2,
+                                    self.composite_multi_ref.c.oid],
+                    **kw
+                )
+
     def _assert_non_simple_warning(self, fn):
         assert_raises_message(
             exc.SAWarning,
@@ -768,6 +794,17 @@ class ColumnCollectionsTest(_JoinFixtures, fixtures.TestBase,
             set([self.three_tab_b.c.id, self.three_tab_b.c.aid])
         )
 
+    def test_determine_local_remote_overlapping_composite_fks(self):
+        joincond = self._join_fixture_overlapping_composite_fks()
+
+        eq_(
+            joincond.local_remote_pairs,
+            [
+                (self.composite_target.c.uid, self.composite_multi_ref.c.uid2,),
+                (self.composite_target.c.oid, self.composite_multi_ref.c.oid,)
+            ]
+        )
+
 class DirectionTest(_JoinFixtures, fixtures.TestBase, AssertsCompiledSQL):
     def test_determine_direction_compound_2(self):
         joincond = self._join_fixture_compound_expression_2(