From: Mike Bayer Date: Sun, 18 Oct 2009 21:59:54 +0000 (+0000) Subject: - the behavior of eagerloading such that the main query is X-Git-Tag: rel_0_6beta1~243 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4bd8451046e7ab7c95455eaf7aaedf8c10afb848;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - the behavior of eagerloading such that the main query is wrapped in a subquery when LIMIT/OFFSET are present now makes an exception for the case when all eager loads are many-to-one joins. In those cases, the eager joins are against the parent table directly along with the limit/offset without the extra overhead of a subquery, since a many-to-one join does not add rows to the result. --- diff --git a/CHANGES b/CHANGES index 2f09040686..4de7b2e22f 100644 --- a/CHANGES +++ b/CHANGES @@ -17,30 +17,54 @@ CHANGES 3.1, Jython2.5. - orm - - the 'expire' option on query.update() has been renamed to - 'fetch', thus matching that of query.delete() - - - query.update() and query.delete() both default to - 'evaluate' for the synchronize strategy. - - - the 'synchronize' strategy for update() and delete() raises - an error on failure. There is no implicit fallback onto - "fetch". Failure of evaluation is based on the structure of - criteria, so success/failure is deterministic based on code - structure. - + - Changes to query.update() and query.delete(): + - the 'expire' option on query.update() has been renamed to + 'fetch', thus matching that of query.delete() + + - query.update() and query.delete() both default to + 'evaluate' for the synchronize strategy. + + - the 'synchronize' strategy for update() and delete() + raises an error on failure. There is no implicit fallback + onto "fetch". Failure of evaluation is based on the + structure of criteria, so success/failure is deterministic + based on code structure. + + - Enhancements on many-to-one relations: + - many-to-one relations now fire off a lazyload in fewer + cases, including in most cases will not fetch the "old" + value when a new one is replaced. + + - many-to-one relation to a joined-table subclass now uses + get() for a simple load (known as the "use_get" + condition), i.e. Related->Sub(Base), without the need to + redefine the primaryjoin condition in terms of the base + table. [ticket:1186] + + - specifying a foreign key with a declarative column, i.e. + ForeignKey(MyRelatedClass.id) doesn't break the "use_get" + condition from taking place [ticket:1492] + + - relation(), eagerload(), and eagerload_all() now feature + an option called "innerjoin". Specify `True` or `False` to + control whether an eager join is constructed as an INNER + or OUTER join. Default is `False` as always. The mapper + options will override whichever setting is specified on + relation(). Should generally be set for many-to-one, not + nullable foreign key relations to allow improved join + performance. [ticket:1544] + + - the behavior of eagerloading such that the main query is + wrapped in a subquery when LIMIT/OFFSET are present now + makes an exception for the case when all eager loads are + many-to-one joins. In those cases, the eager joins are + against the parent table directly along with the + limit/offset without the extra overhead of a subquery, + since a many-to-one join does not add rows to the result. + - the "named tuple" objects returned when iterating a Query() are now pickleable. - - added a flag to relation(), eagerload(), and eagerload_all() - called 'innerjoin'. Specify `True` or `False` to control - whether an eager join is constructed as an INNER or OUTER - join. Default is `False` as always. The mapper options - will override whichever setting is specified on relation(). - Should generally be set for many-to-one, not nullable - foreign key relations to allow improved join performance. - [ticket:1544] - - mapping to a select() construct now requires that you make an alias() out of it distinctly. This to eliminate confusion over such issues as [ticket:1542] @@ -68,10 +92,6 @@ CHANGES None is when comparing an object/collection-referencing attribute within query.filter(), filter_by(), etc. [ticket:1415] - - - many-to-one relations now fire off a lazyload in fewer - cases, including in most cases will not fetch the "old" - value when a new one is replaced. - added "make_transient()" helper function which transforms a persistent/ detached instance into a transient one (i.e. @@ -84,16 +104,6 @@ CHANGES columns will be considered an identity. The need for this scenario typically only occurs when mapping to an outer join. [ticket:1339] - - - many-to-one "lazyload" fixes: - - many-to-one relation to a joined-table subclass now - uses get() for a simple load (known as the "use_get" - condition), i.e. Related->Sub(Base), without the need - to redefine the primaryjoin condition in terms of the - base table. [ticket:1186] - - specifying a foreign key with a declarative column, - i.e. ForeignKey(MyRelatedClass.id) doesn't break the - "use_get" condition from taking place [ticket:1492] - the mechanics of "backref" have been fully merged into the finer grained "back_populates" system, and take place entirely diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 9573b06749..07d7fc64e3 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -1769,13 +1769,19 @@ class Query(object): for entity in self._entities: entity.setup_context(self, context) - + + for rec in context.create_eager_joins: + strategy = rec[0] + strategy(*rec[1:]) + eager_joins = context.eager_joins.values() if context.from_clause: - froms = list(context.from_clause) # "load from explicit FROMs" mode, i.e. when select_from() or join() is used + froms = list(context.from_clause) # "load from explicit FROMs" mode, + # i.e. when select_from() or join() is used else: - froms = context.froms # "load from discrete FROMs" mode, i.e. when each _MappedEntity has its own FROM + froms = context.froms # "load from discrete FROMs" mode, + # i.e. when each _MappedEntity has its own FROM self._adjust_for_single_inheritance(context) @@ -1786,7 +1792,7 @@ class Query(object): else: raise sa_exc.InvalidRequestError("Query contains no columns with which to SELECT from.") - if eager_joins and self._should_nest_selectable: + if context.multi_row_eager_loaders and self._should_nest_selectable: # for eager joins present and LIMIT/OFFSET/DISTINCT, wrap the query inside a select, # then append eager joins onto that @@ -1837,7 +1843,7 @@ class Query(object): order_by_col_expr = list(chain(*[sql_util.find_columns(o) for o in context.order_by])) context.primary_columns += order_by_col_expr - froms += context.eager_joins.values() + froms += tuple(context.eager_joins.values()) statement = sql.select( context.primary_columns + context.secondary_columns, @@ -2004,7 +2010,7 @@ class _MapperEntity(_QueryEntity): def setup_context(self, query, context): adapter = self._get_entity_clauses(query, context) - context.froms.append(self.selectable) + context.froms += (self.selectable,) if context.order_by is False and self.mapper.order_by: context.order_by = self.mapper.order_by @@ -2125,7 +2131,7 @@ class _ColumnEntity(_QueryEntity): def setup_context(self, query, context): column = self._resolve_expr_against_query_aliases(query, self.column, context) - context.froms += list(self.froms) + context.froms += tuple(self.froms) context.primary_columns.append(column) def __str__(self): @@ -2134,6 +2140,10 @@ class _ColumnEntity(_QueryEntity): log.class_logger(Query) class QueryContext(object): + multi_row_eager_loaders = False + adapter = None + froms = () + def __init__(self, query): if query._statement is not None: @@ -2146,8 +2156,6 @@ class QueryContext(object): self.from_clause = query._from_obj self.whereclause = query._criterion self.order_by = query._order_by - if self.order_by: - self.order_by = [expression._literal_as_text(o) for o in util.to_list(self.order_by)] self.query = query self.session = query.session @@ -2157,13 +2165,9 @@ class QueryContext(object): self.primary_columns = [] self.secondary_columns = [] self.eager_order_by = [] - self.enable_eagerloads = query._enable_eagerloads self.eager_joins = {} - self.froms = [] - self.adapter = None - - self.options = set(query._with_options) - self.propagate_options = self.options.difference(o for o in self.options if not o.propagate_to_loaders) + self.create_eager_joins = [] + self.propagate_options = set(o for o in query._with_options if o.propagate_to_loaders) self.attributes = query._attributes.copy() class AliasOption(interfaces.MapperOption): diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index a9fca91796..5ee99ac136 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -626,7 +626,7 @@ class EagerLoader(AbstractRelationLoader): def setup_query(self, context, entity, path, adapter, column_collection=None, parentmapper=None, **kwargs): """Add a left outer join to the statement thats being constructed.""" - if not context.enable_eagerloads: + if not context.query._enable_eagerloads: return path = path + (self.key,) @@ -644,64 +644,77 @@ class EagerLoader(AbstractRelationLoader): add_to_collection = context.primary_columns else: - clauses = self._create_eager_join(context, entity, path, adapter, parentmapper) - if not clauses: - return + # check for join_depth or basic recursion, + # if the current path was not explicitly stated as + # a desired "loaderstrategy" (i.e. via query.options()) + if ("loaderstrategy", path) not in context.attributes: + if self.join_depth: + if len(path) / 2 > self.join_depth: + return + else: + if self.mapper.base_mapper in path: + return + + clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), + equivalents=self.mapper._equivalent_columns, adapt_required=True) + + if self.parent_property.direction != interfaces.MANYTOONE: + context.multi_row_eager_loaders = True + + context.create_eager_joins.append( + (self._create_eager_join, context, entity, path, adapter, parentmapper, clauses) + ) + add_to_collection = context.secondary_columns context.attributes[("eager_row_processor", path)] = clauses - add_to_collection = context.secondary_columns - for value in self.mapper._iterate_polymorphic_properties(): - value.setup(context, entity, path + (self.mapper.base_mapper,), clauses, parentmapper=self.mapper, column_collection=add_to_collection) + value.setup( + context, + entity, + path + (self.mapper.base_mapper,), + clauses, + parentmapper=self.mapper, + column_collection=add_to_collection) - def _create_eager_join(self, context, entity, path, adapter, parentmapper): - # check for join_depth or basic recursion, - # if the current path was not explicitly stated as - # a desired "loaderstrategy" (i.e. via query.options()) - if ("loaderstrategy", path) not in context.attributes: - if self.join_depth: - if len(path) / 2 > self.join_depth: - return - else: - if self.mapper.base_mapper in path: - return - + def _create_eager_join(self, context, entity, path, adapter, parentmapper, clauses): + if parentmapper is None: localparent = entity.mapper else: localparent = parentmapper # whether or not the Query will wrap the selectable in a subquery, - # and then attach eager load joins to that (i.e., in the case of LIMIT/OFFSET etc.) - should_nest_selectable = context.query._should_nest_selectable - + # and then attach eager load joins to that (i.e., in the case of + # LIMIT/OFFSET etc.) + should_nest_selectable = context.multi_row_eager_loaders and \ + context.query._should_nest_selectable + entity_key = None if entity not in context.eager_joins and \ not should_nest_selectable and \ context.from_clause: - index, clause = sql_util.find_join_source(context.from_clause, entity.selectable) + index, clause = \ + sql_util.find_join_source(context.from_clause, entity.selectable) if clause is not None: # join to an existing FROM clause on the query. # key it to its list index in the eager_joins dict. # Query._compile_context will adapt as needed and append to the # FROM clause of the select(). entity_key, default_towrap = index, clause + if entity_key is None: entity_key, default_towrap = entity, entity.selectable towrap = context.eager_joins.setdefault(entity_key, default_towrap) - # create AliasedClauses object to build up the eager query. - clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), - equivalents=self.mapper._equivalent_columns, adapt_required=True) - join_to_left = False if adapter: if getattr(adapter, 'aliased_class', None): onclause = getattr(adapter.aliased_class, self.key, self.parent_property) else: - onclause = getattr(mapperutil.AliasedClass(self.parent, adapter.selectable), self.key, self.parent_property) + onclause = getattr(mapperutil.AliasedClass(self.parent, adapter.selectable), + self.key, self.parent_property) if onclause is self.parent_property: # TODO: this is a temporary hack to account for polymorphic eager loads where @@ -709,9 +722,10 @@ class EagerLoader(AbstractRelationLoader): join_to_left = True else: onclause = self.parent_property - - innerjoin = context.attributes.get(("eager_join_type", path), self.parent_property.innerjoin) - + + innerjoin = context.attributes.get(("eager_join_type", path), + self.parent_property.innerjoin) + context.eager_joins[entity_key] = eagerjoin = mapperutil.join( towrap, clauses.aliased_class, @@ -719,15 +733,17 @@ class EagerLoader(AbstractRelationLoader): join_to_left=join_to_left, isouter=not innerjoin ) - + # send a hint to the Query as to where it may "splice" this join eagerjoin.stop_on = entity.selectable - - if self.parent_property.secondary is None and context.query._should_nest_selectable and not parentmapper: + + if self.parent_property.secondary is None and \ + not parentmapper: # for parentclause that is the non-eager end of the join, # ensure all the parent cols in the primaryjoin are actually in the - # columns clause (i.e. are not deferred), so that aliasing applied by the Query propagates - # those columns outward. This has the effect of "undefering" those columns. + # columns clause (i.e. are not deferred), so that aliasing applied + # by the Query propagates those columns outward. This has the effect + # of "undefering" those columns. for col in sql_util.find_columns(self.parent_property.primaryjoin): if localparent.mapped_table.c.contains_column(col): if adapter: @@ -735,9 +751,12 @@ class EagerLoader(AbstractRelationLoader): context.primary_columns.append(col) if self.parent_property.order_by: - context.eager_order_by += eagerjoin._target_adapter.copy_and_process(util.to_list(self.parent_property.order_by)) - - return clauses + context.eager_order_by += \ + eagerjoin._target_adapter.\ + copy_and_process( + util.to_list(self.parent_property.order_by) + ) + def _create_eager_adapter(self, context, row, adapter, path): if ("user_defined_eager_row_processor", path) in context.attributes: diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index f70e9c3b8d..8618a7fe8e 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -173,6 +173,15 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): Address(id=5, user=User(id=9))] ) + sess.expunge_all() + a = sess.query(Address).filter(Address.id==1).all()[0] + def go(): + eq_(a.user_id, 7) + # assert that the eager loader added 'user_id' to the row and deferred + # loading of that col was disabled + self.assert_sql_count(testing.db, go, 0) + + sess.expunge_all() a = sess.query(Address).filter(Address.id==1).first() def go(): eq_(a.user_id, 7) @@ -563,6 +572,93 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): email_address=u'jack@bean.com',id=7) ) + @testing.resolve_artifact_names + def test_manytoone_limit(self): + """test that the subquery wrapping only occurs with limit/offset and m2m or o2m joins present.""" + + mapper(User, users, properties={ + 'orders':relation(Order, backref='user') + }) + mapper(Order, orders, properties={ + 'items':relation(Item, secondary=order_items, backref='orders'), + 'address':relation(Address) + }) + mapper(Address, addresses) + mapper(Item, items) + + sess = create_session() + + self.assert_compile( + sess.query(User).options(eagerload(User.orders)).limit(10), + "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name AS anon_1_users_name, " + "orders_1.id AS orders_1_id, orders_1.user_id AS orders_1_user_id, orders_1.address_id AS " + "orders_1_address_id, orders_1.description AS orders_1_description, orders_1.isopen AS orders_1_isopen " + "FROM (SELECT users.id AS users_id, users.name AS users_name " + "FROM users " + " LIMIT 10) AS anon_1 LEFT OUTER JOIN orders AS orders_1 ON anon_1.users_id = orders_1.user_id" + ,use_default_dialect=True + ) + + self.assert_compile( + sess.query(Order).options(eagerload(Order.user)).limit(10), + "SELECT orders.id AS orders_id, orders.user_id AS orders_user_id, orders.address_id AS " + "orders_address_id, orders.description AS orders_description, orders.isopen AS orders_isopen, " + "users_1.id AS users_1_id, users_1.name AS users_1_name FROM orders LEFT OUTER JOIN users AS " + "users_1 ON users_1.id = orders.user_id LIMIT 10" + ,use_default_dialect=True + ) + + self.assert_compile( + sess.query(Order).options(eagerload(Order.user, innerjoin=True)).limit(10), + "SELECT orders.id AS orders_id, orders.user_id AS orders_user_id, orders.address_id AS " + "orders_address_id, orders.description AS orders_description, orders.isopen AS orders_isopen, " + "users_1.id AS users_1_id, users_1.name AS users_1_name FROM orders JOIN users AS " + "users_1 ON users_1.id = orders.user_id LIMIT 10" + ,use_default_dialect=True + ) + + self.assert_compile( + sess.query(User).options(eagerload_all("orders.address")).limit(10), + "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name AS anon_1_users_name, " + "addresses_1.id AS addresses_1_id, addresses_1.user_id AS addresses_1_user_id, " + "addresses_1.email_address AS addresses_1_email_address, orders_1.id AS orders_1_id, " + "orders_1.user_id AS orders_1_user_id, orders_1.address_id AS orders_1_address_id, " + "orders_1.description AS orders_1_description, orders_1.isopen AS orders_1_isopen FROM " + "(SELECT users.id AS users_id, users.name AS users_name FROM users LIMIT 10) AS anon_1 " + "LEFT OUTER JOIN orders AS orders_1 ON anon_1.users_id = orders_1.user_id LEFT OUTER JOIN " + "addresses AS addresses_1 ON addresses_1.id = orders_1.address_id" + ,use_default_dialect=True + ) + + self.assert_compile( + sess.query(User).options(eagerload_all("orders.items"), eagerload("orders.address")), + "SELECT users.id AS users_id, users.name AS users_name, items_1.id AS items_1_id, " + "items_1.description AS items_1_description, addresses_1.id AS addresses_1_id, " + "addresses_1.user_id AS addresses_1_user_id, addresses_1.email_address AS " + "addresses_1_email_address, orders_1.id AS orders_1_id, orders_1.user_id AS " + "orders_1_user_id, orders_1.address_id AS orders_1_address_id, orders_1.description " + "AS orders_1_description, orders_1.isopen AS orders_1_isopen FROM users LEFT OUTER JOIN " + "orders AS orders_1 ON users.id = orders_1.user_id LEFT OUTER JOIN order_items AS " + "order_items_1 ON orders_1.id = order_items_1.order_id LEFT OUTER JOIN items AS " + "items_1 ON items_1.id = order_items_1.item_id LEFT OUTER JOIN addresses AS " + "addresses_1 ON addresses_1.id = orders_1.address_id" + ,use_default_dialect=True + ) + + self.assert_compile( + sess.query(User).options(eagerload("orders"), eagerload("orders.address", innerjoin=True)).limit(10), + "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name AS anon_1_users_name, " + "addresses_1.id AS addresses_1_id, addresses_1.user_id AS addresses_1_user_id, " + "addresses_1.email_address AS addresses_1_email_address, orders_1.id AS orders_1_id, " + "orders_1.user_id AS orders_1_user_id, orders_1.address_id AS orders_1_address_id, " + "orders_1.description AS orders_1_description, orders_1.isopen AS orders_1_isopen " + "FROM (SELECT users.id AS users_id, users.name AS users_name " + "FROM users " + " LIMIT 10) AS anon_1 LEFT OUTER JOIN orders AS orders_1 ON anon_1.users_id = " + "orders_1.user_id JOIN addresses AS addresses_1 ON addresses_1.id = orders_1.address_id" + ,use_default_dialect=True + ) + @testing.resolve_artifact_names def test_one_to_many_scalar(self): mapper(User, users, properties = dict(