From 9349ef38efad07157c163afa387820c870ce97e9 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 24 Jan 2007 23:37:03 +0000 Subject: [PATCH] - calling corresponding_column with keys_ok matches columns on name, not key, since the name is meaningful with regards to SQL relationships, the key is not - adjustments to the recent polymorphic relationship refactorings, specifically for many-to-one relationships to polymorphic unions that did not contain the base table [ticket:439]. the lazy/eager clause adaption to the selectable will match up on straight column names (i.e. its a more liberal policy) - lazy loader will not attempt to adapt the clause to the selectable if loads_polymorphic is not enabled, since the more liberal policy of adapting columns fails for more elaborate join conditions - will have to see if ppl want to do complex joins with polymorphic relations... may have to add "polymorphic_primaryjoin" in that case as a last resort (would make working around these issues a snap, tho...) --- CHANGES | 6 ++ lib/sqlalchemy/orm/strategies.py | 9 ++- lib/sqlalchemy/sql.py | 4 +- lib/sqlalchemy/sql_util.py | 4 +- test/orm/inheritance5.py | 108 +++++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+), 7 deletions(-) diff --git a/CHANGES b/CHANGES index 203522c074..32945649a7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,10 +1,16 @@ 0.3.5 +- orm: + - adjustments to the recent polymorphic relationship refactorings, specifically + for many-to-one relationships to polymorphic unions that did not contain the + base table [ticket:439]. the lazy/eager clause adaption to the selectable + will match up on straight column names (i.e. its a more liberal policy) - oracle: - when returning "rowid" as the ORDER BY column or in use with ROW_NUMBER OVER, oracle dialect checks the selectable its being applied to and will switch to table PK if not applicable, i.e. for a UNION. checking for DISTINCT, GROUP BY (other places that rowid is invalid) still a TODO. allows polymorphic mappings to function, [ticket:436] + 0.3.4 - general: - global "insure"->"ensure" change. in US english "insure" is actually diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 2a33c4ff07..af52cd4e95 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -299,11 +299,14 @@ class LazyLoader(AbstractRelationLoader): if secondaryjoin is not None: secondaryjoin = secondaryjoin.copy_container() - secondaryjoin.accept_visitor(sql_util.ClauseAdapter(select_table)) + if self.loads_polymorphic: + secondaryjoin.accept_visitor(sql_util.ClauseAdapter(select_table)) lazywhere = sql.and_(lazywhere, secondaryjoin) else: - lazywhere.accept_visitor(sql_util.ClauseAdapter(select_table)) - + if self.loads_polymorphic: + lazywhere.accept_visitor(sql_util.ClauseAdapter(select_table)) + + print "LAZY CLAUSE", self.key, str(select_table), str(lazywhere) LazyLoader.logger.info("create_lazy_clause " + str(lazywhere)) return (lazywhere, binds, reverse) diff --git a/lib/sqlalchemy/sql.py b/lib/sqlalchemy/sql.py index 5b960546e6..5598472615 100644 --- a/lib/sqlalchemy/sql.py +++ b/lib/sqlalchemy/sql.py @@ -732,7 +732,7 @@ class FromClause(Selectable): """given a ColumnElement, return the ColumnElement object from this Selectable which corresponds to that original Column via a proxy relationship.""" if require_exact: - if self.columns.get(column.key) is column: + if self.columns.get(column.name) is column: return column else: if not raiseerr: @@ -747,7 +747,7 @@ class FromClause(Selectable): else: if keys_ok: try: - return self.c[column.key] + return self.c[column.name] except KeyError: pass if not raiseerr: diff --git a/lib/sqlalchemy/sql_util.py b/lib/sqlalchemy/sql_util.py index 4c6cd4d073..6b87a2dec3 100644 --- a/lib/sqlalchemy/sql_util.py +++ b/lib/sqlalchemy/sql_util.py @@ -139,11 +139,11 @@ class ClauseAdapter(sql.ClauseVisitor): self.selectable = selectable def visit_binary(self, binary): if isinstance(binary.left, sql.ColumnElement): - col = self.selectable.corresponding_column(binary.left, raiseerr=False, keys_ok=False) + col = self.selectable.corresponding_column(binary.left, raiseerr=False, keys_ok=True) if col is not None: binary.left = col if isinstance(binary.right, sql.ColumnElement): - col = self.selectable.corresponding_column(binary.right, raiseerr=False, keys_ok=False) + col = self.selectable.corresponding_column(binary.right, raiseerr=False, keys_ok=True) if col is not None: binary.right = col diff --git a/test/orm/inheritance5.py b/test/orm/inheritance5.py index 7d59c5d547..af1a4918ce 100644 --- a/test/orm/inheritance5.py +++ b/test/orm/inheritance5.py @@ -193,6 +193,114 @@ class RelationTest3(testbase.AssertMixin): assert len(p.colleagues) == 1 assert p.colleagues == [p2] +class RelationTest4(testbase.AssertMixin): + def setUpAll(self): + global metadata, people, engineers, managers, cars + metadata = BoundMetaData(testbase.db) + people = Table('people', metadata, + Column('person_id', Integer, primary_key=True), + Column('name', String(50))) + + engineers = Table('engineers', metadata, + Column('person_id', Integer, ForeignKey('people.person_id'), primary_key=True), + Column('status', String(30))) + + managers = Table('managers', metadata, + Column('person_id', Integer, ForeignKey('people.person_id'), primary_key=True), + Column('longer_status', String(70))) + + cars = Table('cars', metadata, + Column('car_id', Integer, primary_key=True), + Column('owner', Integer, ForeignKey('people.person_id'))) + metadata.create_all() + def tearDownAll(self): + metadata.drop_all() + def tearDown(self): + clear_mappers() + for t in metadata.table_iterator(reverse=True): + t.delete().execute() + + def testmanytoonepolymorphic(self): + """in this test, the polymorphic union is between two subclasses, but does not include the base table by itself + in the union. however, the primaryjoin condition is going to be against the base table, and its a many-to-one + relationship (unlike the test in polymorph.py) so the column in the base table is explicit. Can the ClauseAdapter + figure out how to alias the primaryjoin to the polymorphic union ?""" + # class definitions + class Person(object): + def __init__(self, **kwargs): + for key, value in kwargs.iteritems(): + setattr(self, key, value) + def __repr__(self): + return "Ordinary person %s" % self.name + class Engineer(Person): + def __repr__(self): + return "Engineer %s, status %s" % (self.name, self.status) + class Manager(Person): + def __repr__(self): + return "Manager %s, status %s" % (self.name, self.longer_status) + class Car(object): + def __init__(self, **kwargs): + for key, value in kwargs.iteritems(): + setattr(self, key, value) + def __repr__(self): + return "Car number %d" % self.car_id + + # create a union that represents both types of joins. + employee_join = polymorphic_union( + { + 'engineer':people.join(engineers), + 'manager':people.join(managers), + }, "type", 'employee_join') + + person_mapper = mapper(Person, people, select_table=employee_join,polymorphic_on=employee_join.c.type, polymorphic_identity='person') + engineer_mapper = mapper(Engineer, engineers, inherits=person_mapper, polymorphic_identity='engineer') + manager_mapper = mapper(Manager, managers, inherits=person_mapper, polymorphic_identity='manager') + car_mapper = mapper(Car, cars, properties= {'employee':relation(person_mapper)}) + + # so the primaryjoin is "people.c.person_id==cars.c.owner". the "lazy" clause will be + # "people.c.person_id=?". the employee_join is two selects union'ed together, one of which + # will contain employee.c.person_id the other contains manager.c.person_id. people.c.person_id is not explicitly in + # either column clause in this case. we can modify polymorphic_union to always put the "base" column in which would fix this, + # but im not sure if that really fixes the issue in all cases and its too far from the problem. + # instead, when the primaryjoin is adapted to point to the polymorphic union and is targeting employee_join.c.person_id, + # it has to use not just straight column correspondence but also "keys_ok=True", meaning it will link up to any column + # with the name "person_id", as opposed to columns that descend directly from people.c.person_id. polymorphic unions + # require the cols all match up on column name which then determine the top selectable names, so matching by name is OK. + + session = create_session() + + # creating 5 managers named from M1 to E5 + for i in range(1,5): + session.save(Manager(name="M%d" % i,longer_status="YYYYYYYYY")) + # creating 5 engineers named from E1 to E5 + for i in range(1,5): + session.save(Engineer(name="E%d" % i,status="X")) + + session.flush() + + engineer4 = session.query(Engineer).selectfirst_by(name="E4") + + car1 = Car(owner=engineer4.person_id) + session.save(car1) + session.flush() + + session.clear() + + car1 = session.query(Car).get(car1.car_id) + usingGet = session.query(person_mapper).get(car1.owner) + usingProperty = car1.employee + + # All print should output the same person (engineer E4) + assert str(engineer4) == "Engineer E4, status X" + assert str(usingGet) == "Engineer E4, status X" + assert str(usingProperty) == "Engineer E4, status X" + + session.clear() + + # and now for the lightning round, eager ! + car1 = session.query(Car).options(eagerload('employee')).get(car1.car_id) + assert str(car1.employee) == "Engineer E4, status X" + if __name__ == "__main__": testbase.main() \ No newline at end of file -- 2.47.2