--- /dev/null
+.. change::
+ :tags: bug, orm
+ :tickets: 6887
+
+ Revised the means by which the
+ :attr:`_orm.ORMExecuteState.user_defined_options` accessor receives
+ :class:`_orm.UserDefinedOption` and related option objects from the
+ context, with particular emphasis on the "selectinload" on the loader
+ strategy where this previously was not working; other strategies did not
+ have this problem. The objects that are associated with the current query
+ being executed, and not that of a query being cached, are now propagated
+ unconditionally. This essentially separates them out from the "loader
+ strategy" options which are explicitly associated with the compiled state
+ of a query and need to be used in relation to the cached query.
+
+ The effect of this fix is that a user-defined option, such as those used
+ by the dogpile.caching example as well as for other recipes such as
+ defining a "shard id" for the horizontal sharing extension, will be
+ correctly propagated to eager and lazy loaders regardless of whether
+ a cached query was ultimately invoked.
+
from .interfaces import NOT_EXTENSION
from .interfaces import ONETOMANY
from .interfaces import PropComparator
+from .interfaces import UserDefinedOption
from .loading import merge_frozen_result
from .loading import merge_result
from .mapper import class_mapper
loadopt,
):
- if orig_query is context.query:
- options = new_options = orig_query._with_options
- else:
- # There's currently no test that exercises the necessity of
- # this step for subqueryload. Added in #6881, it is necessary for
- # selectinload, but its necessity for subqueryload is still
- # theoretical.
- options = orig_query._with_options
-
- new_options = [
- orig_opt._adjust_for_extra_criteria(context)
- if orig_opt._is_strategy_option
- else orig_opt
- for orig_opt in options
- ]
+ # note that because the subqueryload object
+ # does not re-use the cached query, instead always making
+ # use of the current invoked query, while we have two queries
+ # here (orig and context.query), they are both non-cached
+ # queries and we can transfer the options as is without
+ # adjusting for new criteria. Some work on #6881 / #6889
+ # brought this into question.
+ new_options = orig_query._with_options
if loadopt and loadopt._extra_criteria:
if orig_query is context.query:
options = new_options = orig_query._with_options
+ user_defined_options = []
else:
options = orig_query._with_options
+ # propagate compile state options from the original query,
+ # updating their "extra_criteria" as necessary.
# note this will create a different cache key than
# "orig" options if extra_criteria is present, because the copy
# of extra_criteria will have different boundparam than that of
if orig_opt._is_strategy_option
else orig_opt
for orig_opt in options
+ if orig_opt._is_compile_state or orig_opt._is_legacy_option
+ ]
+
+ # propagate user defined options from the current query
+ user_defined_options = [
+ opt
+ for opt in context.query._with_options
+ if not opt._is_compile_state and not opt._is_legacy_option
]
if loadopt and loadopt._extra_criteria:
q = q.options(*new_options)._update_compile_options(
{"_current_path": effective_path}
)
+ if user_defined_options:
+ q = q.options(*user_defined_options)
if context.populate_existing:
q = q.execution_options(populate_existing=True)
def fixture_session(**kw):
kw.setdefault("autoflush", True)
kw.setdefault("expire_on_commit", True)
- sess = sa.orm.Session(config.db, **kw)
+
+ bind = kw.pop("bind", config.db)
+
+ sess = sa.orm.Session(bind, **kw)
_fixture_sessions.add(sess)
return sess
from sqlalchemy.orm import events
from sqlalchemy.orm import EXT_SKIP
from sqlalchemy.orm import instrumentation
+from sqlalchemy.orm import joinedload
+from sqlalchemy.orm import lazyload
from sqlalchemy.orm import Mapper
from sqlalchemy.orm import mapper
from sqlalchemy.orm import mapperlib
from sqlalchemy.orm import Session
from sqlalchemy.orm import sessionmaker
from sqlalchemy.orm import subqueryload
+from sqlalchemy.orm import UserDefinedOption
+from sqlalchemy.sql.traversals import NO_CACHE
from sqlalchemy.testing import assert_raises
from sqlalchemy.testing import assert_raises_message
from sqlalchemy.testing import AssertsCompiledSQL
),
)
+ @testing.combinations(
+ ("fixed",), ("payload",), ("dont_cache"), argnames="key_type"
+ )
+ @testing.combinations(
+ (lazyload, 10),
+ (selectinload, 3),
+ (joinedload, 1),
+ (subqueryload, 3),
+ argnames="loader_opt, num_opts",
+ )
+ @testing.combinations((True,), (False,), argnames="use_query_cache")
+ def test_user_option_propagation(
+ self, key_type, loader_opt, num_opts, use_query_cache
+ ):
+ """test #6887.
+
+ The one case here which would break before the bug was fixed is:
+
+ use_query_cache=True, loader_opt=selectinload, key_type="fixed"
+
+ Meaning with a fixed cache key and query caching in place, the user
+ defined loader option would be cached under the same name each time,
+ leading to use of the stale option when using selectinload, as this
+ strategy transfers query options from the cached ORM select into the
+ one that it generates. no other strategy currently does this.
+
+ """
+ User, Address, Dinagling = self.classes("User", "Address", "Dingaling")
+
+ class SetShardOption(UserDefinedOption):
+ propagate_to_loaders = True
+
+ def _gen_cache_key(self, anon_map, bindparams):
+ if key_type == "fixed":
+ return (None,)
+ elif key_type == "payload":
+ return (self.payload,)
+ elif key_type == "dont_cache":
+ anon_map[NO_CACHE] = True
+ return None
+ else:
+ assert False
+
+ if use_query_cache:
+ s = fixture_session()
+ else:
+ s = fixture_session(
+ bind=testing.db.execution_options(compiled_cache=None)
+ )
+
+ m1 = Mock()
+
+ @event.listens_for(s, "do_orm_execute")
+ def go(context):
+ for elem in context.user_defined_options:
+ if isinstance(elem, SetShardOption):
+ m1.update_execution_options(_sa_shard_id=elem.payload)
+
+ stmt = select(User).options(
+ loader_opt(User.addresses).options(loader_opt(Address.dingaling)),
+ SetShardOption("some_shard"),
+ )
+ for u in s.execute(stmt).unique().scalars():
+ for a in u.addresses:
+ a.dingaling
+
+ s.close()
+
+ stmt = select(User).options(
+ loader_opt(User.addresses).options(loader_opt(Address.dingaling)),
+ SetShardOption("some_other_shard"),
+ )
+ for u in s.execute(stmt).unique().scalars():
+ for a in u.addresses:
+ a.dingaling
+ eq_(
+ m1.mock_calls,
+ (
+ [call.update_execution_options(_sa_shard_id="some_shard")]
+ * num_opts
+ )
+ + (
+ [
+ call.update_execution_options(
+ _sa_shard_id="some_other_shard"
+ )
+ ]
+ * num_opts
+ ),
+ )
+
def test_chained_events_one(self):
sess = Session(testing.db, future=True)