- clause adaption hits _raw_columns of a select() (though no ORM tests need this...

- broke up adapter chaining in eagerload, erroneous "wrapping" in row_decorator.  column_property() subqueries are now affected only by the ORMAdapter for that mapper.  fixes [ticket:1037], and may possibly impact some of [ticket:949]

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 660ed35a824ebd2ffacc8337f4044889510274d0..f92363ffe2ca32156528a2f9e192ca40f6dc42a3 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -576,8 +576,7 @@ class EagerLoader(AbstractRelationLoader):
             clauses = self.clauses[path_key]
         except KeyError:
             self.clauses[path_key] = clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), 
-                    equivalents=self.mapper._equivalent_columns, 
-                    chain_to=adapter)
+                    equivalents=self.mapper._equivalent_columns)
 
         if adapter:
             if getattr(adapter, 'aliased_class', None):
@@ -616,11 +615,6 @@ class EagerLoader(AbstractRelationLoader):
                 self.logger.debug("Could not locate aliased clauses for key: " + str(path))
             return False
 
-        if adapter and decorator:
-            decorator = adapter.wrap(decorator)
-        elif adapter:
-            decorator = adapter
-            
         try:
             identity_key = self.mapper.identity_key_from_row(row, decorator)
             return decorator
@@ -632,6 +626,7 @@ class EagerLoader(AbstractRelationLoader):
 
     def create_row_processor(self, context, path, mapper, row, adapter):
         path = path + (self.key,)
+            
         eager_adapter = self.__create_eager_adapter(context, row, adapter, path)
         
         if eager_adapter is not False:
@@ -711,9 +706,6 @@ class LoadEagerFromAliasOption(PropertyOption):
 
                 prop = mapper.get_property(propname, resolve_synonyms=True)
                 self.alias = prop.target.alias(self.alias)
-            if not isinstance(self.alias, expression.Alias):
-                import pdb
-                pdb.set_trace()
             query._attributes[("eager_row_processor", paths[-1])] = sql_util.ColumnAdapter(self.alias)
         else:
             query._attributes[("eager_row_processor", paths[-1])] = None

diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 7ce6377011289920bc164e6d4495b23b65c29bc5..9d48b31c1edd452cfb5fe1f1f267ca3f789f3bcd 100644 (file)
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -3066,7 +3066,7 @@ class Select(_SelectBaseMixin, FromClause):
         """return child elements as per the ClauseElement specification."""
 
         return (column_collections and list(self.columns) or []) + \
-            list(self._froms) + \
+            self._raw_columns + list(self._froms) + \
             [x for x in (self._whereclause, self._having, self._order_by_clause, self._group_by_clause) if x is not None]
 
     def column(self, column):

diff --git a/test/orm/query.py b/test/orm/query.py
index 9f326005a7d2b70450b023330809e8cbf915b471..46a3cbacd250233134ce57dc91b51b8ee71224d9 100644 (file)
--- a/test/orm/query.py
+++ b/test/orm/query.py
@@ -1869,7 +1869,7 @@ class ExternalColumnsTest(QueryTest):
         })
         clear_mappers()
 
-    def test_external_columns_good(self):
+    def test_external_columns(self):
         """test querying mappings that reference external columns or selectables."""
         
         mapper(User, users, properties={
@@ -1957,6 +1957,34 @@ class ExternalColumnsTest(QueryTest):
             [(1, 7, 14, 1), (2, 8, 16, 3), (3, 8, 16, 3), (4, 8, 16, 3), (5, 9, 18, 1)]
         )
 
+    def test_external_columns_eagerload(self):
+        # in this test, we have a subquery on User that accesses "addresses", underneath
+        # an eagerload for "addresses".  So the "addresses" alias adapter needs to *not* hit 
+        # the "addresses" table within the "user" subquery, but "user" still needs to be adapted.
+        # therefore the long standing practice of eager adapters being "chained" has been removed
+        # since its unnecessary and breaks this exact condition.
+        mapper(User, users, properties={
+            'addresses':relation(Address, backref='user', order_by=addresses.c.id),
+            'concat': column_property((users.c.id * 2)),
+            'count': column_property(select([func.count(addresses.c.id)], users.c.id==addresses.c.user_id).correlate(users))
+        })
+        mapper(Address, addresses)
+        mapper(Order, orders, properties={
+            'address':relation(Address),  # m2o
+        })
+
+        sess = create_session()
+        def go():
+            o1 = sess.query(Order).options(eagerload_all('address.user')).get(1)
+            self.assertEquals(o1.address.user.count, 1)
+        self.assert_sql_count(testing.db, go, 1)
+
+        sess = create_session()
+        def go():
+            o1 = sess.query(Order).options(eagerload_all('address.user')).first()
+            self.assertEquals(o1.address.user.count, 1)
+        self.assert_sql_count(testing.db, go, 1)
+
 
 if __name__ == '__main__':
     testenv.main()

diff --git a/test/sql/generative.py b/test/sql/generative.py
index cf5ea8235213684b8d1761619bb64894de2494f2..cb5406b754a2e7769862b36ac9896781feb6ff5f 100644 (file)
--- a/test/sql/generative.py
+++ b/test/sql/generative.py
@@ -532,6 +532,12 @@ class ClauseAdapterTest(TestBase, AssertsCompiledSQL):
             "(SELECT table1.col1 AS col1, table1.col2 AS col2, table1.col3 AS col3 FROM table1) AS foo  LIMIT 5 OFFSET 10) AS anon_1 "\
             "LEFT OUTER JOIN table1 AS bar ON anon_1.col1 = bar.col1")
     
+    def test_functions(self):
+        self.assert_compile(sql_util.ClauseAdapter(t1.alias()).traverse(func.count(t1.c.col1)), "count(table1_1.col1)")
+
+        s = select([func.count(t1.c.col1)])
+        self.assert_compile(sql_util.ClauseAdapter(t1.alias()).traverse(s), "SELECT count(table1_1.col1) AS count_1 FROM table1 AS table1_1")
+        
     def test_recursive(self):
         metadata = MetaData()
         a = Table('a', metadata,