From: Mike Bayer Date: Thu, 18 Jan 2007 03:33:13 +0000 (+0000) Subject: - some deeper error checking when compiling relations, to detect an ambiguous "prima... X-Git-Tag: rel_0_3_4~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bfbf626317f8bb05fd19759f6d05f54fb7e2b3f9;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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 --- diff --git a/CHANGES b/CHANGES index 0dc9411c84..d8cff55dec 100644 --- a/CHANGES +++ b/CHANGES @@ -46,6 +46,9 @@ - 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 diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index fc945fc723..ef7c823600 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -274,12 +274,17 @@ class PropertyLoader(StrategizedProperty): 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 @@ -298,7 +303,7 @@ class PropertyLoader(StrategizedProperty): 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): diff --git a/test/orm/relationships.py b/test/orm/relationships.py index e0bb5fecd8..ef8ee9e02b 100644 --- a/test/orm/relationships.py +++ b/test/orm/relationships.py @@ -351,6 +351,74 @@ class RelationTest3(testbase.PersistTest): 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