From e02114ce4975714454feacc953cb955aaedb6596 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 12 May 2008 16:15:28 +0000 Subject: [PATCH] - clause adaption hits _raw_columns of a select() (though no ORM tests need this feature currently) - 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] --- lib/sqlalchemy/orm/strategies.py | 12 ++---------- lib/sqlalchemy/sql/expression.py | 2 +- test/orm/query.py | 30 +++++++++++++++++++++++++++++- test/sql/generative.py | 6 ++++++ 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 660ed35a82..f92363ffe2 100644 --- 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 7ce6377011..9d48b31c1e 100644 --- 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 9f326005a7..46a3cbacd2 100644 --- 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 cf5ea82352..cb5406b754 100644 --- 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, -- 2.47.3