- 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...)

5 files changed, 124 insertions, 7 deletions

diff --git a/CHANGES b/CHANGES
index 203522c07..32945649a 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 2a33c4ff0..af52cd4e9 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 5b960546e..559847261 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 4c6cd4d07..6b87a2dec 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 7d59c5d54..af1a4918c 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