- Fixed bug involving read/write relation()s that

contain literal or other non-column expressions
within their primaryjoin condition equated
to a foreign key column.
- fixed UnmappedColumnError exception raise to not assume it was passed a column

diff --git a/CHANGES b/CHANGES
index 56d2ec7254c74acceb9d869e2f9313029fd457d4..6ec64c9c9a6fe51c81e6e991eb3e529c06313d3b 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,11 @@ CHANGES
 0.5.0rc2
 ========
 - orm
+    - Fixed bug involving read/write relation()s that 
+      contain literal or other non-column expressions 
+      within their primaryjoin condition equated
+      to a foreign key column.
+      
     - "non-batch" mode in mapper(), a feature which allows
       mapper extension methods to be called as each instance
       is updated/inserted, now honors the insert order

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 52b90d22a7adbd8a670b4971e58916148c783fd8..e79cd47cb7732357a4d2b87259d1b9fa9b4e99c7 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1012,7 +1012,7 @@ class Mapper(object):
             if prop:
                 raise exc.UnmappedColumnError("Column '%s.%s' is not available, due to conflicting property '%s':%s" % (column.table.name, column.name, column.key, repr(prop)))
             else:
-                raise exc.UnmappedColumnError("No column %s.%s is configured on mapper %s..." % (column.table.name, column.name, str(self)))
+                raise exc.UnmappedColumnError("No column %s is configured on mapper %s..." % (column, self))
 
     # TODO: improve names
     def _get_state_attr_by_column(self, state, column):

diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 190b9dc0c2654852526992728c8bcaba127d1e64..92adc19cde9eac96200b0c5cb78db8d5fb8f51b9 100644 (file)
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -628,7 +628,7 @@ class PropertyLoader(StrategizedProperty):
                     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 r in self._foreign_keys]
+            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):

diff --git a/test/orm/relationships.py b/test/orm/relationships.py
index fcb1807d675a47f47c520f1f933eeb054af10423..549bb0c169923282a35902fb6b841324dba8ef09 100644 (file)
--- a/test/orm/relationships.py
+++ b/test/orm/relationships.py
@@ -598,7 +598,58 @@ class RelationTest5(_base.MappedTest):
         for old, new in zip(con.lineItems, newcon.lineItems):
             assert old.id == new.id
 
+class RelationTest6(_base.MappedTest):
+    """test a relation with a non-column entity in the primary join, 
+    is not viewonly, and also has the non-column's clause mentioned in the 
+    foreign keys list.
+    
+    """
+    
+    def define_tables(self, metadata):
+        Table('tags', metadata, Column("id", Integer, primary_key=True),
+            Column("data", Integer),
+        )
+
+        Table('tag_foo', metadata, 
+            Column("id", Integer, primary_key=True),
+            Column('tagid', Integer),
+            Column("data", Integer),
+        )
 
+    @testing.resolve_artifact_names
+    def test_basic(self):
+        class Tag(_base.ComparableEntity):
+            pass
+        class TagInstance(_base.ComparableEntity):
+            pass
+
+        mapper(Tag, tags, properties={
+            'foo':relation(TagInstance, 
+               primaryjoin=sa.and_(tag_foo.c.data=='iplc_case',
+                                tag_foo.c.tagid==tags.c.id),
+               foreign_keys=[tag_foo.c.tagid, tag_foo.c.data],
+               ),
+        })
+
+        mapper(TagInstance, tag_foo)
+
+        sess = create_session()
+        t1 = Tag(data='some tag')
+        t1.foo.append(TagInstance(data='iplc_case'))
+        t1.foo.append(TagInstance(data='not_iplc_case'))
+        sess.add(t1)
+        sess.flush()
+        sess.clear()
+        
+        # relation works
+        eq_(sess.query(Tag).all(), [Tag(data='some tag', foo=[TagInstance(data='iplc_case')])])
+        
+        # both TagInstances were persisted
+        eq_(
+            sess.query(TagInstance).order_by(TagInstance.data).all(), 
+            [TagInstance(data='iplc_case'), TagInstance(data='not_iplc_case')]
+        )
+        
 class TypeMatchTest(_base.MappedTest):
     """test errors raised when trying to add items whose type is not handled by a relation"""