From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 29 Jul 2008 19:49:46 +0000 (+0000)
Subject: relation.order_by requires _literal_as_column conversion as well
X-Git-Tag: rel_0_5beta3~17
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=449b33e0590fb40a34222718326b189429d622ae;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

relation.order_by requires _literal_as_column conversion as well
---

diff --git a/CHANGES b/CHANGES
index 100626f1a2..4af45f143f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -59,6 +59,9 @@ CHANGES
     - Class-bound attributes sent as arguments to 
       relation()'s remote_side and foreign_keys parameters 
       are now accepted, allowing them to be used with declarative.
+      Additionally fixed bugs involving order_by being
+      specified as a class-bound attribute in conjunction
+      with eager loading.
       
 - mysql
     - Quoting of MSEnum values for use in CREATE TABLE is now
diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index 18e816546d..41e191312e 100644
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -16,7 +16,6 @@ import inspect, StringIO
 from sqlalchemy import exc, schema, util, types, log
 from sqlalchemy.sql import expression
 
-
 class Dialect(object):
     """Define the behavior of a specific database and DB-API combination.
 
@@ -1497,7 +1496,6 @@ class ResultProxy(object):
     def _create_key_cache(self):
         # local copies to avoid circular ref against 'self'
         props = self.__props
-        context = self.context
         def lookup_key(key):
             """Given a key, which could be a ColumnElement, string, etc.,
             matches it to the appropriate key we got from the result set's
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 51e0631e50..8c6a92a1eb 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -533,6 +533,9 @@ class PropertyLoader(StrategizedProperty):
             if callable(getattr(self, attr)):
                 setattr(self, attr, getattr(self, attr)())
         
+        if self.order_by:
+            self.order_by = [expression._literal_as_column(x) for x in util.to_list(self.order_by)]
+        
         self._foreign_keys = set(expression._literal_as_column(x) for x in util.to_set(self._foreign_keys))
         self.remote_side = set(expression._literal_as_column(x) for x in util.to_set(self.remote_side))
 
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index fbfcd51c6b..944fc9c5dd 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -9,7 +9,6 @@
 import weakref
 
 import sqlalchemy.exceptions as sa_exc
-import sqlalchemy.orm.attributes
 from sqlalchemy import util, sql, engine
 from sqlalchemy.sql import util as sql_util, expression
 from sqlalchemy.orm import (
@@ -902,6 +901,7 @@ class Session(object):
         """Return a new ``Query`` object corresponding to this ``Session``."""
 
         return self._query_cls(entities, self, **kwargs)
+
     def _autoflush(self):
         if self.autoflush and (self.transaction is None or self.transaction.autoflush):
             self.flush()
@@ -941,6 +941,7 @@ class Session(object):
 
     def expire_all(self):
         """Expires all persistent instances within this Session."""
+
         for state in self.identity_map.all_states():
             _expire_state(state, None)
 
diff --git a/test/ext/declarative.py b/test/ext/declarative.py
index 6cf553a375..31f0dfd2d8 100644
--- a/test/ext/declarative.py
+++ b/test/ext/declarative.py
@@ -2,8 +2,8 @@ import testenv; testenv.configure_for_tests()
 
 from sqlalchemy.ext import declarative as decl
 from testlib import sa, testing
-from testlib.sa import MetaData, Table, Column, Integer, String, ForeignKey, ForeignKeyConstraint
-from testlib.sa.orm import relation, create_session, class_mapper
+from testlib.sa import MetaData, Table, Column, Integer, String, ForeignKey, ForeignKeyConstraint, asc
+from testlib.sa.orm import relation, create_session, class_mapper, eagerload
 from testlib.testing import eq_
 from orm._base import ComparableEntity
 
@@ -205,7 +205,37 @@ class DeclarativeTest(testing.TestBase, testing.AssertsExecutionResults):
         a1 = sess.query(Address).filter(Address.email == 'two').one()
         eq_(a1, Address(email='two'))
         eq_(a1.user, User(name='u1'))
+    
+    def test_eager_order_by(self):
+        class Address(Base, ComparableEntity):
+            __tablename__ = 'addresses'
+
+            id = Column('id', Integer, primary_key=True)
+            email = Column('email', String(50))
+            user_id = Column('user_id', Integer, ForeignKey('users.id'))
 
+        class User(Base, ComparableEntity):
+            __tablename__ = 'users'
+
+            id = Column('id', Integer, primary_key=True)
+            name = Column('name', String(50))
+            addresses = relation("Address", order_by=Address.email)
+
+        Base.metadata.create_all()
+        u1 = User(name='u1', addresses=[
+            Address(email='two'),
+            Address(email='one'),
+        ])
+        sess = create_session()
+        sess.save(u1)
+        sess.flush()
+        sess.clear()
+        eq_(sess.query(User).options(eagerload(User.addresses)).all(), [User(name='u1', addresses=[
+            Address(email='one'),
+            Address(email='two'),
+        ])])
+
+            
     def test_as_declarative(self):
         class User(ComparableEntity):
             __tablename__ = 'users'