From: Mike Bayer Date: Thu, 27 Mar 2014 23:43:15 +0000 (-0400) Subject: - Improved the check for "how to join from A to B" such that when X-Git-Tag: rel_0_9_4~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0611baa889198421afa932f2af1524bd8826ed7d;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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 f16dd4f89a..f2594b7333 100644 --- 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 d59b45fae5..7a220f9492 100644 --- 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 10ba41429d..0b35a44eb3 100644 --- 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(