Commits

Mike Bayer  committed 9a12706

- some deeper error checking when compiling relations, to detect an ambiguous "primaryjoin"
in the case that both sides of the relationship have foreign key references in the primary
join condition

  • Participants
  • Parent commits f60d10c

Comments (0)

Files changed (3)

   - poked the first hole in the can of worms: saying query.select_by(somerelationname=someinstance)
   will create the join of the primary key columns represented by "somerelationname"'s mapper to the
   actual primary key in "someinstance".
+  - some deeper error checking when compiling relations, to detect an ambiguous "primaryjoin"
+  in the case that both sides of the relationship have foreign key references in the primary
+  join condition
   - added a mutex to the mapper compilation step.  ive been reluctant to add any kind
   of threading anything to SA but this is one spot that its its really needed since mappers
   are typically "global", and while their state does not change during normal operation, the 

File lib/sqlalchemy/orm/properties.py

                         return sync.ONETOMANY
                 else:
                     return sync.MANYTOONE
-        elif len([c for c in self.foreignkey if self.mapper.unjoined_table.corresponding_column(c, False) is not None]):
-            return sync.ONETOMANY
-        elif len([c for c in self.foreignkey if self.parent.unjoined_table.corresponding_column(c, False) is not None]):
-            return sync.MANYTOONE
         else:
-            raise exceptions.ArgumentError("Cant determine relation direction for '%s' in mapper '%s' with primary join\n '%s'" %(self.key, str(self.mapper), str(self.primaryjoin)))
+            onetomany = len([c for c in self.foreignkey if self.mapper.unjoined_table.corresponding_column(c, False) is not None])
+            manytoone = len([c for c in self.foreignkey if self.parent.unjoined_table.corresponding_column(c, False) is not None])
+            if not onetomany and not manytoone:
+                raise exceptions.ArgumentError("Cant determine relation direction for '%s' on mapper '%s' with primary join '%s' - foreign key columns are not present in neither the parent nor the child's mapped tables" %(self.key, str(self.parent), str(self.primaryjoin)))
+            elif onetomany and manytoone:
+                raise exceptions.ArgumentError("Cant determine relation direction for '%s' on mapper '%s' with primary join '%s' - foreign key columns are present in both the parent and the child's mapped tables.  Specify 'foreignkey' argument." %(self.key, str(self.parent), str(self.primaryjoin)))
+            elif onetomany:
+                return sync.ONETOMANY
+            elif manytoone:
+                return sync.MANYTOONE
             
     def _find_dependent(self):
         """searches through the primary join condition to determine which side
         visitor = mapperutil.BinaryVisitor(foo)
         self.primaryjoin.accept_visitor(visitor)
         if len(foreignkeys) == 0:
-            raise exceptions.ArgumentError("On relation '%s', can't figure out which side is the foreign key for join condition '%s'.  Specify the 'foreignkey' argument to the relation." % (self.key, str(self.primaryjoin)))
+            raise exceptions.ArgumentError("Cant determine relation direction for '%s' on mapper '%s' with primary join '%s' - no foreign key relationship is expressed within the join condition.  Specify 'foreignkey' argument." %(self.key, str(self.parent), str(self.primaryjoin)))
         self.foreignkey = foreignkeys
         
     def get_join(self):

File test/orm/relationships.py

         s.delete(j)
         s.flush()
         
+class CyclicalRelationInheritTest(testbase.PersistTest):
+    """testing a mapper with an inheriting mapper, where the base mapper also has a relation to the inheriting mapper,
+    as well as using unexpected columns to create the join conditions which do not indicate the usual "cyclical" relationship
+    this represents.  test that proper error messages are raised guiding the user to a reasonably working setup, and that 
+    the ultimate setup works."""
+    def setUpAll(self):
+        global people, managers, metadata
+        metadata = BoundMetaData(testbase.db)
+
+        people = Table('people', metadata, 
+           Column('person_id', Integer, Sequence('person_id_seq', optional=True), primary_key=True),
+           Column('manager_id', Integer, ForeignKey('managers.person_id', use_alter=True, name="mpid_fq")),
+           Column('name', String(50)),
+           Column('type', String(30)))
+
+        managers = Table('managers', metadata, 
+           Column('person_id', Integer, ForeignKey('people.person_id'), primary_key=True),
+           Column('status', String(30)),
+           Column('manager_name', String(50))
+           )
+
+        metadata.create_all()
+
+    def tearDownAll(self):
+        metadata.drop_all()
+
+    def tearDown(self):
+        clear_mappers()
+        people.update().execute(manager_id=None)
+        for t in metadata.table_iterator(reverse=True):
+            t.delete().execute()
+
+    def testbasic(self):
+        class Person(object):
+            pass
+        class Manager(Person):
+            pass
+
+        mapper(Person, people, properties={
+            'manager':relation(Manager, primaryjoin=people.c.manager_id==managers.c.person_id, uselist=False)
+        })
+        mapper(Manager, managers, inherits=Person, inherit_condition=people.c.person_id==managers.c.person_id)
+        try:
+            compile_mappers()
+        except exceptions.ArgumentError, ar:
+            assert str(ar) == "Cant determine relation direction for 'manager' on mapper 'Mapper|Person|people' with primary join 'people.manager_id = managers.person_id' - foreign key columns are present in both the parent and the child's mapped tables.  Specify 'foreignkey' argument."
+
+        clear_mappers()
+
+        mapper(Person, people, properties={
+            'manager':relation(Manager, primaryjoin=people.c.manager_id==managers.c.person_id, foreignkey=people.c.manager_id, uselist=False, post_update=True)
+        })
+        mapper(Manager, managers, inherits=Person, inherit_condition=people.c.person_id==managers.c.person_id)
+
+        session = create_session()
+        p = Person()
+        p.name = 'some person'
+        m = Manager()
+        m.name = 'some manager'
+        p.manager = m
+        session.save(p)
+        session.flush()
+        session.clear()
+
+        p = session.query(Person).get(p.person_id)
+        m = session.query(Manager).get(m.person_id)
+        print p, m, p.manager
+        assert p.manager is m