From: Mike Bayer Date: Tue, 26 May 2020 02:36:44 +0000 (-0400) Subject: Small callcount reductions and refinement for cached queries X-Git-Tag: rel_1_4_0b1~301 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=903b18828461bb8cb8dca4acc56809b3df2b14d5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Small callcount reductions and refinement for cached queries baked wasn't using the new one()/first()/one_or_none() methods, fixed that. loading._instance_processor() can skip setting up the quick populators every time because it can cache the getters. Callcounts have gone below what 1.3 does for the test_baked_query performance suite, however runtime for continued inexplicable reasons has not :(. still suspecting the result tuples but this seems so hard to believe. Change-Id: Ifbca04834d27350e0fa82cb8512e66112abc8729 --- diff --git a/lib/sqlalchemy/ext/baked.py b/lib/sqlalchemy/ext/baked.py index 112e245f78..fdcaec5595 100644 --- a/lib/sqlalchemy/ext/baked.py +++ b/lib/sqlalchemy/ext/baked.py @@ -383,7 +383,7 @@ class Result(object): return str(self._as_query()) def __iter__(self): - return iter(self._iter()) + return self._iter().__iter__() def _iter(self): bq = self.bq @@ -463,16 +463,15 @@ class Result(object): Equivalent to :meth:`_query.Query.first`. """ + bq = self.bq.with_criteria(lambda q: q.slice(0, 1)) - ret = list( + return ( bq.for_session(self.session) .params(self._params) ._using_post_criteria(self._post_criteria) + ._iter() + .first() ) - if len(ret) > 0: - return ret[0] - else: - return None def one(self): """Return exactly one result or raise an exception. @@ -480,19 +479,7 @@ class Result(object): Equivalent to :meth:`_query.Query.one`. """ - try: - ret = self.one_or_none() - except orm_exc.MultipleResultsFound as err: - util.raise_( - orm_exc.MultipleResultsFound( - "Multiple rows were found for one()" - ), - replace_context=err, - ) - else: - if ret is None: - raise orm_exc.NoResultFound("No row was found for one()") - return ret + return self._iter().one() def one_or_none(self): """Return one or zero results, or raise an exception for multiple @@ -503,17 +490,7 @@ class Result(object): .. versionadded:: 1.0.9 """ - ret = list(self) - - l = len(ret) - if l == 1: - return ret[0] - elif l == 0: - return None - else: - raise orm_exc.MultipleResultsFound( - "Multiple rows were found for one_or_none()" - ) + return self._iter().one_or_none() def all(self): """Return all rows. diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py index 616e757a39..102e74b997 100644 --- a/lib/sqlalchemy/orm/loading.py +++ b/lib/sqlalchemy/orm/loading.py @@ -14,8 +14,6 @@ as well as some of the attribute loading strategies. """ from __future__ import absolute_import -import collections - from . import attributes from . import exc as orm_exc from . import path_registry @@ -588,48 +586,86 @@ def _instance_processor( identity_class = mapper._identity_class - populators = collections.defaultdict(list) + compile_state = context.compile_state + + populators = {} props = mapper._prop_set if only_load_props is not None: props = props.intersection(mapper._props[k] for k in only_load_props) - quick_populators = path.get( - context.attributes, "memoized_setups", _none_set - ) + getters = path.get(compile_state.attributes, "getters", None) + if getters is None: + # directives given to us by the ColumnLoader.setup_query() + # methods. Turn these directives into getters against the + # actual result set. + quick_populators = path.get( + context.attributes, "memoized_setups", _none_set + ) + cached_populators = { + "new": [], + "expire": [], + "quick": [], + "todo": [], + "delayed": [], + "existing": [], + "eager": [], + } - for prop in props: - if prop in quick_populators: - # this is an inlined path just for column-based attributes. - col = quick_populators[prop] - if col is _DEFER_FOR_STATE: - populators["new"].append( - (prop.key, prop._deferred_column_loader) - ) - elif col is _SET_DEFERRED_EXPIRED: - # note that in this path, we are no longer - # searching in the result to see if the column might - # be present in some unexpected way. - populators["expire"].append((prop.key, False)) - elif col is _RAISE_FOR_STATE: - populators["new"].append((prop.key, prop._raise_column_loader)) - else: - getter = None - if not getter: - getter = result._getter(col, False) - if getter: - populators["quick"].append((prop.key, getter)) - else: - # fall back to the ColumnProperty itself, which - # will iterate through all of its columns - # to see if one fits - prop.create_row_processor( - context, path, mapper, result, adapter, populators + pk_cols = mapper.primary_key + + if adapter: + pk_cols = [adapter.columns[c] for c in pk_cols] + getters = { + "populators": cached_populators, + "primary_key_getter": result._tuple_getter(pk_cols), + } + + for prop in props: + if prop in quick_populators: + # this is an inlined path just for column-based attributes. + col = quick_populators[prop] + if col is _DEFER_FOR_STATE: + cached_populators["new"].append( + (prop.key, prop._deferred_column_loader) ) - else: - prop.create_row_processor( - context, path, mapper, result, adapter, populators - ) + elif col is _SET_DEFERRED_EXPIRED: + # note that in this path, we are no longer + # searching in the result to see if the column might + # be present in some unexpected way. + cached_populators["expire"].append((prop.key, False)) + elif col is _RAISE_FOR_STATE: + cached_populators["new"].append( + (prop.key, prop._raise_column_loader) + ) + else: + getter = None + if not getter: + getter = result._getter(col, False) + if getter: + cached_populators["quick"].append((prop.key, getter)) + else: + # fall back to the ColumnProperty itself, which + # will iterate through all of its columns + # to see if one fits + prop.create_row_processor( + context, + path, + mapper, + result, + adapter, + cached_populators, + ) + else: + cached_populators["todo"].append(prop) + path.set(compile_state.attributes, "getters", getters) + + cached_populators = getters["populators"] + populators = {k: list(v) for k, v in cached_populators.items()} + for prop in cached_populators["todo"]: + prop.create_row_processor( + context, path, mapper, result, adapter, populators + ) propagated_loader_options = context.propagated_loader_options load_path = ( @@ -707,11 +743,7 @@ def _instance_processor( else: refresh_identity_key = None - pk_cols = mapper.primary_key - - if adapter: - pk_cols = [adapter.columns[c] for c in pk_cols] - tuple_getter = result._tuple_getter(pk_cols) + primary_key_getter = getters["primary_key_getter"] if mapper.allow_partial_pks: is_not_primary_key = _none_set.issuperset @@ -732,7 +764,11 @@ def _instance_processor( else: # look at the row, see if that identity is in the # session, or we have to create a new one - identitykey = (identity_class, tuple_getter(row), identity_token) + identitykey = ( + identity_class, + primary_key_getter(row), + identity_token, + ) instance = session_identity_map.get(identitykey) @@ -875,7 +911,7 @@ def _instance_processor( def ensure_no_pk(row): identitykey = ( identity_class, - tuple_getter(row), + primary_key_getter(row), identity_token, ) if not is_not_primary_key(identitykey[1]): diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py index ecb5e3919b..8b9fe8c116 100644 --- a/test/ext/test_baked.py +++ b/test/ext/test_baked.py @@ -180,7 +180,7 @@ class LikeQueryTest(BakedTest): assert_raises_message( orm_exc.MultipleResultsFound, - "Multiple rows were found for one_or_none()", + "Multiple rows were found when one or none was required", bq(Session()).one_or_none, ) @@ -192,7 +192,7 @@ class LikeQueryTest(BakedTest): assert_raises_message( orm_exc.NoResultFound, - "No row was found for one()", + "No row was found when one was required", bq(Session()).one, ) @@ -213,7 +213,7 @@ class LikeQueryTest(BakedTest): assert_raises_message( orm_exc.MultipleResultsFound, - "Multiple rows were found for one()", + "Multiple rows were found when exactly one was required", bq(Session()).one, )