From: Mike Bayer Date: Sat, 14 Aug 2010 18:52:18 +0000 (-0400) Subject: - Another pass through the series of error messages X-Git-Tag: rel_0_6_4~38 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8fc50cd3a4eb98501c5ae47ead07c20b485934df;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Another pass through the series of error messages emitted when relationship() is configured with ambiguous arguments. The "foreign_keys" setting is no longer mentioned, as it is almost never needed and it is preferable users set up correct ForeignKey metadata, which is now the recommendation. If 'foreign_keys' is used and is incorrect, the message suggests the attribute is probably unnecessary. Docs for the attribute are beefed up. This because all confused relationship() users on the ML appear to be attempting to use foreign_keys due to the message, which only confuses them further since Table metadata is much clearer. - If the "secondary" table has no ForeignKey metadata and no foreign_keys is set, even though the user is passing screwed up information, it is assumed that primary/secondaryjoin expressions should consider only and all cols in "secondary" to be foreign. It's not possible with "secondary" for the foreign keys to be elsewhere in any case. A warning is now emitted instead of an error, and the mapping succeeds. [ticket:1877] - fixed incorrect "Alternate Collection Mappings" reference in the docs, not sure if someone wants to reference "Rows that Point to Themselves" function - "Collection Mapping" is "Advanced Collection Mapping", this section is troublesome since nobody really needs it but it is public API --- diff --git a/CHANGES b/CHANGES index da9f3dca0e..4f60857ef8 100644 --- a/CHANGES +++ b/CHANGES @@ -21,6 +21,31 @@ CHANGES with_polymorphic selectable, instead of silently ignoring it. Look for this to become an exception in 0.7. + + - Another pass through the series of error messages + emitted when relationship() is configured with + ambiguous arguments. The "foreign_keys" + setting is no longer mentioned, as it is almost + never needed and it is preferable users set up + correct ForeignKey metadata, which is now the + recommendation. If 'foreign_keys' + is used and is incorrect, the message suggests + the attribute is probably unnecessary. Docs + for the attribute are beefed up. This + because all confused relationship() users on the + ML appear to be attempting to use foreign_keys + due to the message, which only confuses them + further since Table metadata is much clearer. + + - If the "secondary" table has no ForeignKey metadata + and no foreign_keys is set, even though the + user is passing screwed up information, it is assumed + that primary/secondaryjoin expressions should + consider only and all cols in "secondary" to be + foreign. It's not possible with "secondary" for + the foreign keys to be elsewhere in any case. + A warning is now emitted instead of an error, + and the mapping succeeds. [ticket:1877] - Moving an o2m object from one collection to another, or vice versa changing the referenced diff --git a/doc/build/mappers.rst b/doc/build/mappers.rst index 97677aa080..71f71a8cfa 100644 --- a/doc/build/mappers.rst +++ b/doc/build/mappers.rst @@ -1490,7 +1490,6 @@ Theres no restriction on how many times you can relate from parent to child. SQ addresses_table.c.city=='New York')), }) -.. _alternate_collection_implementations: Rows that point to themselves / Mutually Dependent Rows ------------------------------------------------------- @@ -1528,7 +1527,7 @@ To enable the UPDATE after INSERT / UPDATE before DELETE behavior on :func:`~sql When a structure using the above mapping is flushed, the "widget" row will be INSERTed minus the "favorite_entry_id" value, then all the "entry" rows will be INSERTed referencing the parent "widget" row, and then an UPDATE statement will populate the "favorite_entry_id" column of the "widget" table (it's one row at a time for the time being). -.. _advdatamapping_entitycollections: +.. _alternate_collection_implementations: Alternate Collection Implementations ------------------------------------- diff --git a/doc/build/ormtutorial.rst b/doc/build/ormtutorial.rst index 2d6a3325ea..6f38a35c90 100644 --- a/doc/build/ormtutorial.rst +++ b/doc/build/ormtutorial.rst @@ -781,7 +781,7 @@ We'll need to create the ``addresses`` table in the database, so we will issue a Working with Related Objects ============================= -Now when we create a ``User``, a blank ``addresses`` collection will be present. Various collection types, such as sets and dictionaries, are possible here (see :ref:`advdatamapping_entitycollections` for details), but by default, the collection is a Python list. +Now when we create a ``User``, a blank ``addresses`` collection will be present. Various collection types, such as sets and dictionaries, are possible here (see :ref:`alternate_collection_implementations` for details), but by default, the collection is a Python list. .. sourcecode:: python+sql diff --git a/doc/build/reference/orm/collections.rst b/doc/build/reference/orm/collections.rst index 350e631251..818f114989 100644 --- a/doc/build/reference/orm/collections.rst +++ b/doc/build/reference/orm/collections.rst @@ -1,5 +1,5 @@ -Collection Mapping -================== +Advanced Collection Mapping +=========================== This is an in-depth discussion of collection mechanics. For simple examples, see :ref:`alternate_collection_implementations`. diff --git a/lib/sqlalchemy/ext/declarative.py b/lib/sqlalchemy/ext/declarative.py index 2310f01ce3..e40ba3ec4b 100755 --- a/lib/sqlalchemy/ext/declarative.py +++ b/lib/sqlalchemy/ext/declarative.py @@ -1197,7 +1197,7 @@ def _deferred_relationship(cls, prop): if isinstance(prop, RelationshipProperty): for attr in ('argument', 'order_by', 'primaryjoin', 'secondaryjoin', - 'secondary', '_foreign_keys', 'remote_side'): + 'secondary', '_user_defined_foreign_keys', 'remote_side'): v = getattr(prop, attr) if isinstance(v, basestring): setattr(prop, attr, resolve_arg(v)) diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index 2004dccd1e..c74fabacd7 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -274,15 +274,24 @@ def relationship(argument, secondary=None, **kwargs): :param foreign_keys: a list of columns which are to be used as "foreign key" columns. - this parameter should be used in conjunction with explicit - ``primaryjoin`` and ``secondaryjoin`` (if needed) arguments, and - the columns within the ``foreign_keys`` list should be present - within those join conditions. Normally, ``relationship()`` will - inspect the columns within the join conditions to determine - which columns are the "foreign key" columns, based on - information in the ``Table`` metadata. Use this argument when no - ForeignKey's are present in the join condition, or to override - the table-defined foreign keys. + Normally, :func:`relationship` uses the :class:`.ForeignKey` + and :class:`.ForeignKeyConstraint` objects present within the + mapped or secondary :class:`.Table` to determine the "foreign" side of + the join condition. This is used to construct SQL clauses in order + to load objects, as well as to "synchronize" values from + primary key columns to referencing foreign key columns. + The ``foreign_keys`` parameter overrides the notion of what's + "foreign" in the table metadata, allowing the specification + of a list of :class:`.Column` objects that should be considered + part of the foreign key. + + There are only two use cases for ``foreign_keys`` - one, when it is not + convenient for :class:`.Table` metadata to contain its own foreign key + metadata (which should be almost never, unless reflecting a large amount of + tables from a MySQL MyISAM schema, or a schema that doesn't actually + have foreign keys on it). The other is for extremely + rare and exotic composite foreign key setups where some columns + should artificially not be considered as foreign. :param innerjoin=False: when ``True``, joined eager loads will use an inner join to join diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index cbfba91f32..5788c30f94 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -446,7 +446,7 @@ class RelationshipProperty(StrategizedProperty): self.viewonly = viewonly self.lazy = lazy self.single_parent = single_parent - self._foreign_keys = foreign_keys + self._user_defined_foreign_keys = foreign_keys self.collection_class = collection_class self.passive_deletes = passive_deletes self.passive_updates = passive_updates @@ -695,7 +695,7 @@ class RelationshipProperty(StrategizedProperty): if isinstance(other, (NoneType, expression._Null)): if self.property.direction == MANYTOONE: return sql.or_(*[x != None for x in - self.property._foreign_keys]) + self.property._calculated_foreign_keys]) else: return self._criterion_exists() elif self.property.uselist: @@ -912,7 +912,7 @@ class RelationshipProperty(StrategizedProperty): 'primaryjoin', 'secondaryjoin', 'secondary', - '_foreign_keys', + '_user_defined_foreign_keys', 'remote_side', ): if util.callable(getattr(self, attr)): @@ -931,9 +931,9 @@ class RelationshipProperty(StrategizedProperty): if self.order_by is not False and self.order_by is not None: self.order_by = [expression._literal_as_column(x) for x in util.to_list(self.order_by)] - self._foreign_keys = \ + self._user_defined_foreign_keys = \ util.column_set(expression._literal_as_column(x) for x in - util.to_column_set(self._foreign_keys)) + util.to_column_set(self._user_defined_foreign_keys)) self.remote_side = \ util.column_set(expression._literal_as_column(x) for x in util.to_column_set(self.remote_side)) @@ -999,8 +999,8 @@ class RelationshipProperty(StrategizedProperty): raise sa_exc.ArgumentError("Could not determine join " "condition between parent/child tables on " "relationship %s. Specify a 'primaryjoin' " - "expression. If this is a many-to-many " - "relationship, 'secondaryjoin' is needed as well." + "expression. If 'secondary' is present, " + "'secondaryjoin' is needed as well." % self) def _col_is_part_of_mappings(self, column): @@ -1012,91 +1012,121 @@ class RelationshipProperty(StrategizedProperty): self.target.c.contains_column(column) or \ self.secondary.c.contains_column(column) is not None + def _sync_pairs_from_join(self, join_condition, primary): + """Given a join condition, figure out what columns are foreign + and are part of a binary "equated" condition to their referecned + columns, and convert into a list of tuples of (primary col->foreign col). + + Make several attempts to determine if cols are compared using + "=" or other comparators (in which case suggest viewonly), + columns are present but not part of the expected mappings, columns + don't have any :class:`ForeignKey` information on them, or + the ``foreign_keys`` attribute is being used incorrectly. + + """ + eq_pairs = criterion_as_pairs(join_condition, + consider_as_foreign_keys=self._user_defined_foreign_keys, + any_operator=self.viewonly) + + eq_pairs = [(l, r) for (l, r) in eq_pairs + if self._col_is_part_of_mappings(l) + and self._col_is_part_of_mappings(r) + or self.viewonly and r in self._user_defined_foreign_keys] + + if not eq_pairs and \ + self.secondary is not None and \ + not self._user_defined_foreign_keys: + fks = set(self.secondary.c) + eq_pairs = criterion_as_pairs(join_condition, + consider_as_foreign_keys=fks, + any_operator=self.viewonly) + + eq_pairs = [(l, r) for (l, r) in eq_pairs + if self._col_is_part_of_mappings(l) + and self._col_is_part_of_mappings(r) + or self.viewonly and r in fks] + if eq_pairs: + util.warn("No ForeignKey objects were present " + "in secondary table '%s'. Assumed referenced " + "foreign key columns %s for join condition '%s' " + "on relationship %s" % ( + self.secondary.description, + ", ".join(sorted(["'%s'" % col for col in fks])), + join_condition, + self + )) + + if not eq_pairs: + if not self.viewonly and criterion_as_pairs(join_condition, + consider_as_foreign_keys=self._user_defined_foreign_keys, + any_operator=True): + raise sa_exc.ArgumentError("Could not locate any " + "equated, locally mapped column pairs for %s " + "condition '%s' on relationship %s. For more " + "relaxed rules on join conditions, the " + "relationship may be marked as viewonly=True." + % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self + )) + else: + if self._user_defined_foreign_keys: + raise sa_exc.ArgumentError("Could not determine " + "relationship direction for %s condition " + "'%s', on relationship %s, using manual " + "'foreign_keys' setting. Do the columns " + "in 'foreign_keys' represent all, and " + "only, the 'foreign' columns in this join " + "condition? Does the %s Table already " + "have adequate ForeignKey and/or " + "ForeignKeyConstraint objects established " + "(in which case 'foreign_keys' is usually " + "unnecessary)?" + % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self, + primary and 'mapped' or 'secondary' + )) + else: + raise sa_exc.ArgumentError("Could not determine " + "relationship direction for %s condition " + "'%s', on relationship %s. Ensure that the " + "referencing Column objects have a " + "ForeignKey present, or are otherwise part " + "of a ForeignKeyConstraint on their parent " + "Table." + % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self + )) + return eq_pairs + def _determine_synchronize_pairs(self): if self.local_remote_pairs: - if not self._foreign_keys: + if not self._user_defined_foreign_keys: raise sa_exc.ArgumentError('foreign_keys argument is ' 'required with _local_remote_pairs argument') self.synchronize_pairs = [] for l, r in self.local_remote_pairs: - if r in self._foreign_keys: + if r in self._user_defined_foreign_keys: self.synchronize_pairs.append((l, r)) - elif l in self._foreign_keys: + elif l in self._user_defined_foreign_keys: self.synchronize_pairs.append((r, l)) else: - eq_pairs = criterion_as_pairs(self.primaryjoin, - consider_as_foreign_keys=self._foreign_keys, - any_operator=self.viewonly) - eq_pairs = [(l, r) for (l, r) in eq_pairs - if self._col_is_part_of_mappings(l) - and self._col_is_part_of_mappings(r) - or self.viewonly and r in self._foreign_keys] - if not eq_pairs: - if not self.viewonly \ - and criterion_as_pairs(self.primaryjoin, - consider_as_foreign_keys=self._foreign_keys, - any_operator=True): - raise sa_exc.ArgumentError("Could not locate any " - "equated, locally mapped column pairs for " - "primaryjoin condition '%s' on " - "relationship %s. For more relaxed rules " - "on join conditions, the relationship may " - "be marked as viewonly=True." - % (self.primaryjoin, self)) - else: - if self._foreign_keys: - raise sa_exc.ArgumentError("Could not determine" - " relationship direction for " - "primaryjoin condition '%s', on " - "relationship %s. Do the columns in " - "'foreign_keys' represent only the " - "'foreign' columns in this join " - "condition ?" % (self.primaryjoin, - self)) - else: - raise sa_exc.ArgumentError("Could not determine" - " relationship direction for " - "primaryjoin condition '%s', on " - "relationship %s. Specify the " - "'foreign_keys' argument to indicate " - "which columns on the relationship are " - "foreign." % (self.primaryjoin, self)) + eq_pairs = self._sync_pairs_from_join(self.primaryjoin, True) self.synchronize_pairs = eq_pairs if self.secondaryjoin is not None: - sq_pairs = criterion_as_pairs(self.secondaryjoin, - consider_as_foreign_keys=self._foreign_keys, - any_operator=self.viewonly) - sq_pairs = [(l, r) for (l, r) in sq_pairs - if self._col_is_part_of_mappings(l) - and self._col_is_part_of_mappings(r) or r - in self._foreign_keys] - if not sq_pairs: - if not self.viewonly \ - and criterion_as_pairs(self.secondaryjoin, - consider_as_foreign_keys=self._foreign_keys, - any_operator=True): - raise sa_exc.ArgumentError("Could not locate any " - "equated, locally mapped column pairs for " - "secondaryjoin condition '%s' on " - "relationship %s. For more relaxed rules " - "on join conditions, the relationship may " - "be marked as viewonly=True." - % (self.secondaryjoin, self)) - else: - raise sa_exc.ArgumentError("Could not determine " - "relationship direction for secondaryjoin " - "condition '%s', on relationship %s. " - "Specify the foreign_keys argument to " - "indicate which columns on the " - "relationship are foreign." - % (self.secondaryjoin, self)) + sq_pairs = self._sync_pairs_from_join(self.secondaryjoin, False) self.secondary_synchronize_pairs = sq_pairs else: self.secondary_synchronize_pairs = None - self._foreign_keys = util.column_set(r for (l, r) in + self._calculated_foreign_keys = util.column_set(r for (l, r) in self.synchronize_pairs) if self.secondary_synchronize_pairs: - self._foreign_keys.update(r for (l, r) in + self._calculated_foreign_keys.update(r for (l, r) in self.secondary_synchronize_pairs) def _determine_direction(self): @@ -1114,7 +1144,7 @@ class RelationshipProperty(StrategizedProperty): remote = self.remote_side else: remote = None - if not remote or self._foreign_keys.difference(l for (l, + if not remote or self._calculated_foreign_keys.difference(l for (l, r) in self.synchronize_pairs).intersection(remote): self.direction = ONETOMANY else: @@ -1192,12 +1222,12 @@ class RelationshipProperty(StrategizedProperty): eq_pairs += self.secondary_synchronize_pairs else: eq_pairs = criterion_as_pairs(self.primaryjoin, - consider_as_foreign_keys=self._foreign_keys, + consider_as_foreign_keys=self._calculated_foreign_keys, any_operator=True) if self.secondaryjoin is not None: eq_pairs += \ criterion_as_pairs(self.secondaryjoin, - consider_as_foreign_keys=self._foreign_keys, + consider_as_foreign_keys=self._calculated_foreign_keys, any_operator=True) eq_pairs = [(l, r) for (l, r) in eq_pairs if self._col_is_part_of_mappings(l) @@ -1266,7 +1296,7 @@ class RelationshipProperty(StrategizedProperty): "a non-secondary relationship." ) foreign_keys = kwargs.pop('foreign_keys', - self._foreign_keys) + self._user_defined_foreign_keys) parent = self.parent.primary_mapper() kwargs.setdefault('viewonly', self.viewonly) kwargs.setdefault('post_update', self.post_update) diff --git a/lib/sqlalchemy/test/testing.py b/lib/sqlalchemy/test/testing.py index 70ddc7ba20..78cd74d22c 100644 --- a/lib/sqlalchemy/test/testing.py +++ b/lib/sqlalchemy/test/testing.py @@ -533,6 +533,7 @@ def assert_raises_message(except_cls, msg, callable_, *args, **kwargs): assert False, "Callable did not raise an exception" except except_cls, e: assert re.search(msg, str(e)), "%r !~ %s" % (msg, e) + print str(e) def fail(msg): assert False, msg diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index 7f67631a98..ce315ff350 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -1797,6 +1797,13 @@ class InvalidRelationshipEscalationTest(_base.MappedTest): Column('id', Integer, primary_key=True), Column('fid', Integer)) + Table('foos_with_fks', metadata, + Column('id', Integer, primary_key=True), + Column('fid', Integer, ForeignKey('foos_with_fks.id'))) + Table('bars_with_fks', metadata, + Column('id', Integer, primary_key=True), + Column('fid', Integer, ForeignKey('foos_with_fks.id'))) + @classmethod def setup_classes(cls): class Foo(_base.Entity): @@ -1859,11 +1866,20 @@ class InvalidRelationshipEscalationTest(_base.MappedTest): foreign_keys=[foos.c.id, bars.c.fid])}) mapper(Bar, bars) - assert_raises_message( - sa.exc.ArgumentError, - "Do the columns in 'foreign_keys' represent only the " - "'foreign' columns in this join condition ?", - sa.orm.compile_mappers) + assert_raises_message(sa.exc.ArgumentError, + "Could not determine relationship " + "direction for primaryjoin condition " + "'foos.id = bars.fid', on relationship " + "Foo.bars, using manual 'foreign_keys' " + "setting. Do the columns in " + "'foreign_keys' represent all, and only, " + "the 'foreign' columns in this join " + r"condition\? Does the mapped Table " + "already have adequate ForeignKey and/or " + "ForeignKeyConstraint objects " + r"established \(in which case " + r"'foreign_keys' is usually unnecessary\)\?" + , sa.orm.compile_mappers) @testing.resolve_artifact_names def test_ambiguous_remoteside_o2m(self): @@ -1931,11 +1947,19 @@ class InvalidRelationshipEscalationTest(_base.MappedTest): viewonly=True)}) mapper(Bar, bars) - assert_raises_message( - sa.exc.ArgumentError, - "Could not determine relationship direction for primaryjoin condition", - sa.orm.compile_mappers) + assert_raises_message(sa.exc.ArgumentError, + 'Could not determine relationship ' + 'direction for primaryjoin condition', + sa.orm.compile_mappers) + sa.orm.clear_mappers() + mapper(Foo, foos_with_fks, properties={ + 'bars':relationship(Bar, + primaryjoin=foos_with_fks.c.id>bars_with_fks.c.fid, + viewonly=True)}) + mapper(Bar, bars_with_fks) + sa.orm.compile_mappers() + @testing.resolve_artifact_names def test_no_equated_self_ref_viewonly(self): mapper(Foo, foos, properties={ @@ -1944,10 +1968,23 @@ class InvalidRelationshipEscalationTest(_base.MappedTest): viewonly=True)}) mapper(Bar, bars) - assert_raises_message( - sa.exc.ArgumentError, - "Specify the 'foreign_keys' argument to indicate which columns " - "on the relationship are foreign.", sa.orm.compile_mappers) + assert_raises_message(sa.exc.ArgumentError, + "Could not determine relationship " + "direction for primaryjoin condition " + "'foos.id > foos.fid', on relationship " + "Foo.foos. Ensure that the referencing " + "Column objects have a ForeignKey " + "present, or are otherwise part of a " + "ForeignKeyConstraint on their parent " + "Table.", sa.orm.compile_mappers) + + sa.orm.clear_mappers() + mapper(Foo, foos_with_fks, properties={ + 'foos':relationship(Foo, + primaryjoin=foos_with_fks.c.id>foos_with_fks.c.fid, + viewonly=True)}) + mapper(Bar, bars_with_fks) + sa.orm.compile_mappers() @testing.resolve_artifact_names def test_no_equated_self_ref_viewonly_fks(self): @@ -1972,6 +2009,13 @@ class InvalidRelationshipEscalationTest(_base.MappedTest): "Could not determine relationship direction for primaryjoin condition", sa.orm.compile_mappers) + sa.orm.clear_mappers() + mapper(Foo, foos_with_fks, properties={ + 'bars':relationship(Bar, + primaryjoin=foos_with_fks.c.id==bars_with_fks.c.fid)}) + mapper(Bar, bars_with_fks) + sa.orm.compile_mappers() + @testing.resolve_artifact_names def test_equated_self_ref(self): mapper(Foo, foos, properties={ @@ -1982,6 +2026,7 @@ class InvalidRelationshipEscalationTest(_base.MappedTest): sa.exc.ArgumentError, "Could not determine relationship direction for primaryjoin condition", sa.orm.compile_mappers) + @testing.resolve_artifact_names def test_equated_self_ref_wrong_fks(self): @@ -2007,6 +2052,20 @@ class InvalidRelationshipEscalationTestM2M(_base.MappedTest): Table('bars', metadata, Column('id', Integer, primary_key=True)) + Table('foobars_with_fks', metadata, + Column('fid', Integer, ForeignKey('foos.id')), + Column('bid', Integer, ForeignKey('bars.id')) + ) + + Table('foobars_with_many_columns', metadata, + Column('fid', Integer), + Column('bid', Integer), + Column('fid1', Integer), + Column('bid1', Integer), + Column('fid2', Integer), + Column('bid2', Integer), + ) + @classmethod @testing.resolve_artifact_names def setup_classes(cls): @@ -2040,6 +2099,80 @@ class InvalidRelationshipEscalationTestM2M(_base.MappedTest): "on relationship", sa.orm.compile_mappers) + @testing.resolve_artifact_names + def test_no_fks_warning_1(self): + mapper(Foo, foos, properties={ + 'bars': relationship(Bar, secondary=foobars, + primaryjoin=foos.c.id==foobars.c.fid, + secondaryjoin=foobars.c.bid==bars.c.id)}) + mapper(Bar, bars) + + assert_raises_message(sa.exc.SAWarning, + "No ForeignKey objects were present in " + "secondary table 'foobars'. Assumed " + "referenced foreign key columns " + "'foobars.bid', 'foobars.fid' for join " + "condition 'foos.id = foobars.fid' on " + "relationship Foo.bars", + sa.orm.compile_mappers) + + sa.orm.clear_mappers() + mapper(Foo, foos, properties={ + 'bars': relationship(Bar, secondary=foobars_with_many_columns, + primaryjoin=foos.c.id==foobars_with_many_columns.c.fid, + secondaryjoin=foobars_with_many_columns.c.bid==bars.c.id)}) + mapper(Bar, bars) + + assert_raises_message(sa.exc.SAWarning, + "No ForeignKey objects were present in " + "secondary table 'foobars_with_many_colum" + "ns'. Assumed referenced foreign key " + "columns 'foobars_with_many_columns.bid'," + " 'foobars_with_many_columns.bid1', " + "'foobars_with_many_columns.bid2', " + "'foobars_with_many_columns.fid', " + "'foobars_with_many_columns.fid1', " + "'foobars_with_many_columns.fid2' for " + "join condition 'foos.id = " + "foobars_with_many_columns.fid' on " + "relationship Foo.bars", + sa.orm.compile_mappers) + + @testing.emits_warning(r'No ForeignKey objects.*') + @testing.resolve_artifact_names + def test_no_fks_warning_2(self): + mapper(Foo, foos, properties={ + 'bars': relationship(Bar, secondary=foobars, + primaryjoin=foos.c.id==foobars.c.fid, + secondaryjoin=foobars.c.bid==bars.c.id)}) + mapper(Bar, bars) + sa.orm.compile_mappers() + eq_( + Foo.bars.property.synchronize_pairs, + [(foos.c.id, foobars.c.fid)] + ) + eq_( + Foo.bars.property.secondary_synchronize_pairs, + [(bars.c.id, foobars.c.bid)] + ) + + sa.orm.clear_mappers() + mapper(Foo, foos, properties={ + 'bars': relationship(Bar, secondary=foobars_with_many_columns, + primaryjoin=foos.c.id==foobars_with_many_columns.c.fid, + secondaryjoin=foobars_with_many_columns.c.bid==bars.c.id)}) + mapper(Bar, bars) + sa.orm.compile_mappers() + eq_( + Foo.bars.property.synchronize_pairs, + [(foos.c.id, foobars_with_many_columns.c.fid)] + ) + eq_( + Foo.bars.property.secondary_synchronize_pairs, + [(bars.c.id, foobars_with_many_columns.c.bid)] + ) + + @testing.resolve_artifact_names def test_bad_primaryjoin(self): mapper(Foo, foos, properties={ @@ -2053,7 +2186,29 @@ class InvalidRelationshipEscalationTestM2M(_base.MappedTest): sa.exc.ArgumentError, "Could not determine relationship direction for primaryjoin condition", sa.orm.compile_mappers) + + sa.orm.clear_mappers() + mapper(Foo, foos, properties={ + 'bars': relationship(Bar, + secondary=foobars_with_fks, + primaryjoin=foos.c.id > foobars_with_fks.c.fid, + secondaryjoin=foobars_with_fks.c.bid<=bars.c.id)}) + mapper(Bar, bars) + assert_raises_message( + sa.exc.ArgumentError, + "Could not locate any equated, locally mapped column pairs for primaryjoin condition ", + sa.orm.compile_mappers) + sa.orm.clear_mappers() + mapper(Foo, foos, properties={ + 'bars': relationship(Bar, + secondary=foobars_with_fks, + primaryjoin=foos.c.id > foobars_with_fks.c.fid, + secondaryjoin=foobars_with_fks.c.bid<=bars.c.id, + viewonly=True)}) + mapper(Bar, bars) + sa.orm.compile_mappers() + @testing.resolve_artifact_names def test_bad_secondaryjoin(self): mapper(Foo, foos, properties={ @@ -2064,10 +2219,20 @@ class InvalidRelationshipEscalationTestM2M(_base.MappedTest): foreign_keys=[foobars.c.fid])}) mapper(Bar, bars) - assert_raises_message( - sa.exc.ArgumentError, - "Could not determine relationship direction for secondaryjoin " - "condition", sa.orm.compile_mappers) + assert_raises_message(sa.exc.ArgumentError, + "Could not determine relationship " + "direction for secondaryjoin condition " + r"'foobars.bid \<\= bars.id', on " + "relationship Foo.bars, using manual " + "'foreign_keys' setting. Do the columns " + "in 'foreign_keys' represent all, and only, the " + "'foreign' columns in this join " + r"condition\? Does the " + "secondary Table already have adequate " + "ForeignKey and/or ForeignKeyConstraint " + r"objects established \(in which case " + r"'foreign_keys' is usually unnecessary\)?" + , sa.orm.compile_mappers) @testing.resolve_artifact_names def test_no_equated_secondaryjoin(self):