--- /dev/null
+.. change::
+ :tags: bug, orm
+ :tickets: 6881
+
+ Fixed issue in :func:`_orm.selectinload` where use of the new
+ :meth:`_orm.PropComparator.and_` feature within options that were nested
+ more than one level deep would fail to update bound parameter values that
+ were in the nested criteria, as a side effect of SQL statement caching.
+
_is_criteria_option = False
+ _is_strategy_option = False
+
class LoaderOption(ORMOption):
"""Describe a loader modification to an ORM statement at compilation time.
effective_entity,
loadopt,
):
- opts = orig_query._with_options
+
+ 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
+ ]
if loadopt and loadopt._extra_criteria:
- opts += (
+ new_options += (
orm_util.LoaderCriteriaOption(
self.entity,
loadopt._generate_extra_criteria(context),
# propagate loader options etc. to the new query.
# these will fire relative to subq_path.
q = q._with_current_path(rewritten_path)
- q = q.options(*opts)
+ q = q.options(*new_options)
return q
effective_path = path[self.parent_property]
- options = orig_query._with_options
+ if orig_query is context.query:
+ options = new_options = orig_query._with_options
+ else:
+ options = orig_query._with_options
+
+ # 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
+ # the QueryableAttribute in the path
+
+ new_options = [
+ orig_opt._adjust_for_extra_criteria(context)
+ if orig_opt._is_strategy_option
+ else orig_opt
+ for orig_opt in options
+ ]
if loadopt and loadopt._extra_criteria:
- options += (
+ new_options += (
orm_util.LoaderCriteriaOption(
effective_entity,
loadopt._generate_extra_criteria(context),
),
)
- q = q.options(*options)._update_compile_options(
+ q = q.options(*new_options)._update_compile_options(
{"_current_path": effective_path}
)
"""
+ _is_strategy_option = True
+
_cache_key_traversal = [
("path", visitors.ExtendedInternalTraversal.dp_has_cache_key),
("strategy", visitors.ExtendedInternalTraversal.dp_plain_obj),
def _generate_extra_criteria(self, context):
"""Apply the current bound parameters in a QueryContext to the
- "extra_criteria" stored with this Load object.
+ immediate "extra_criteria" stored with this Load object.
Load objects are typically pulled from the cached version of
the statement from a QueryContext. The statement currently being
return k2._apply_params_to_element(k1, and_(*self._extra_criteria))
+ def _adjust_for_extra_criteria(self, context):
+ """Apply the current bound parameters in a QueryContext to all
+ occurrences "extra_criteria" stored within al this Load object;
+ copying in place.
+
+ """
+ orig_query = context.compile_state.select_statement
+
+ applied = {}
+
+ ck = [None, None]
+
+ def process(opt):
+ if not opt._extra_criteria:
+ return
+
+ if ck[0] is None:
+ ck[:] = (
+ orig_query._generate_cache_key(),
+ context.query._generate_cache_key(),
+ )
+ k1, k2 = ck
+
+ opt._extra_criteria = tuple(
+ k2._apply_params_to_element(k1, crit)
+ for crit in opt._extra_criteria
+ )
+
+ return self._deep_clone(applied, process)
+
+ def _deep_clone(self, applied, process):
+ if self in applied:
+ return applied[self]
+
+ cloned = self._generate()
+
+ applied[self] = cloned
+
+ cloned.strategy = self.strategy
+
+ assert cloned.propagate_to_loaders == self.propagate_to_loaders
+ assert cloned.is_class_strategy == self.is_class_strategy
+ assert cloned.is_opts_only == self.is_opts_only
+
+ if self.context:
+ cloned.context = util.OrderedDict(
+ [
+ (
+ key,
+ value._deep_clone(applied, process)
+ if isinstance(value, Load)
+ else value,
+ )
+ for key, value in self.context.items()
+ ]
+ )
+
+ cloned.local_opts.update(self.local_opts)
+
+ process(cloned)
+
+ return cloned
+
@property
def _context_cache_key(self):
serialized = []
else:
return None
- if attr._extra_criteria:
+ if attr._extra_criteria and not self._extra_criteria:
+ # in most cases, the process that brings us here will have
+ # already established _extra_criteria. however if not,
+ # and it's present on the attribute, then use that.
self._extra_criteria = attr._extra_criteria
if getattr(attr, "_of_type", None):
# anonymous clone of the Load / UnboundLoad object since #5056
self._to_bind = None
+ def _deep_clone(self, applied, process):
+ if self in applied:
+ return applied[self]
+
+ cloned = self._generate()
+
+ applied[self] = cloned
+
+ cloned.strategy = self.strategy
+
+ assert cloned.propagate_to_loaders == self.propagate_to_loaders
+ assert cloned.is_class_strategy == self.is_class_strategy
+ assert cloned.is_opts_only == self.is_opts_only
+
+ cloned._to_bind = [
+ elem._deep_clone(applied, process) for elem in self._to_bind or ()
+ ]
+
+ cloned.local_opts.update(self.local_opts)
+
+ process(cloned)
+
+ return cloned
+
def _apply_to_parent(self, parent, applied, bound, to_bind=None):
if self in applied:
return applied[self]
loader.strategy = self.strategy
loader.is_opts_only = self.is_opts_only
loader.is_class_strategy = self.is_class_strategy
+ loader._extra_criteria = self._extra_criteria
path = loader.path
context = execute_observed.context
compare_dialect = self._compile_dialect(execute_observed)
+ # received_statement runs a full compile(). we should not need to
+ # consider extracted_parameters; if we do this indicates some state
+ # is being sent from a previous cached query, which some misbehaviors
+ # in the ORM can cause, see #6881
+ cache_key = None # execute_observed.context.compiled.cache_key
+ extracted_parameters = (
+ None # execute_observed.context.extracted_parameters
+ )
+
if "schema_translate_map" in context.execution_options:
map_ = context.execution_options["schema_translate_map"]
else:
if isinstance(execute_observed.clauseelement, _DDLCompiles):
compiled = execute_observed.clauseelement.compile(
- dialect=compare_dialect, schema_translate_map=map_
+ dialect=compare_dialect,
+ schema_translate_map=map_,
)
else:
compiled = execute_observed.clauseelement.compile(
+ cache_key=cache_key,
dialect=compare_dialect,
column_keys=context.compiled.column_keys,
for_executemany=context.compiled.for_executemany,
parameters = execute_observed.parameters
if not parameters:
- _received_parameters = [compiled.construct_params()]
+ _received_parameters = [
+ compiled.construct_params(
+ extracted_parameters=extracted_parameters
+ )
+ ]
else:
_received_parameters = [
- compiled.construct_params(m) for m in parameters
+ compiled.construct_params(
+ m, extracted_parameters=extracted_parameters
+ )
+ for m in parameters
]
return _received_statement, _received_parameters
def test_lazyload_plus_joined_aliased_abs_bcs(self):
A, B, C = self.classes("A", "B", "C")
+
s = fixture_session()
aa = aliased(A)
q = (
return Order, Item
+ @testing.fixture
+ def user_order_item_fixture(self):
+ User, Order, Item = self.classes("User", "Order", "Item")
+ users, orders, items, order_items = self.tables(
+ "users", "orders", "items", "order_items"
+ )
+
+ mapper(
+ User,
+ users,
+ properties={"orders": relationship(Order, order_by=orders.c.id)},
+ )
+ mapper(
+ Order,
+ orders,
+ properties={
+ # m2m
+ "items": relationship(
+ Item, secondary=order_items, order_by=items.c.id
+ ),
+ },
+ )
+ mapper(Item, items)
+
+ return User, Order, Item
+
@testing.fixture
def mixin_fixture(self):
users = self.tables.users
class RelationshipCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
__dialect__ = "default"
- @testing.fixture
- def user_address_fixture(self):
- users, Address, addresses, User = (
- self.tables.users,
- self.classes.Address,
- self.tables.addresses,
- self.classes.User,
- )
-
- mapper(
- User,
- users,
- properties={
- "addresses": relationship(
- mapper(Address, addresses), order_by=Address.id
- )
- },
- )
- return User, Address
-
def _user_minus_edwood(self, User, Address):
return [
User(
),
)
+ @testing.combinations((True,), (False,), argnames="use_compiled_cache")
+ def test_selectinload_nested_criteria(
+ self, user_order_item_fixture, use_compiled_cache
+ ):
+ User, Order, Item = user_order_item_fixture
+
+ if not use_compiled_cache:
+ s = Session(
+ testing.db.execution_options(compiled_cache=None), future=True
+ )
+ else:
+ s = Session(testing.db, future=True)
+
+ def go(order_description, item_description):
+ stmt = (
+ select(User)
+ .where(User.id == 7)
+ .options(
+ selectinload(
+ User.orders.and_(
+ Order.description == order_description
+ )
+ ).joinedload(
+ Order.items.and_(Item.description == item_description)
+ ),
+ )
+ )
+ return s.execute(stmt)
+
+ for order_description, item_description, oid, iid in (
+ ("order 3", "item 3", 3, 3),
+ ("order 3", "item 4", 3, 4),
+ ("order 3", "item 4", 3, 4),
+ ("order 5", "item 5", 5, 5),
+ ("order 3", "item 3", 3, 3),
+ ("order 5", "item 5", 5, 5),
+ ):
+ s.close()
+ with self.sql_execution_asserter() as asserter:
+ result = go(order_description, item_description)
+
+ eq_(
+ result.scalars().unique().all(),
+ [User(id=7, orders=[Order(id=oid, items=[Item(id=iid)])])],
+ )
+ asserter.assert_(
+ CompiledSQL(
+ "SELECT users.id, users.name FROM users "
+ "WHERE users.id = :id_1",
+ [{"id_1": 7}],
+ ),
+ CompiledSQL(
+ "SELECT orders.user_id AS orders_user_id, "
+ "orders.id AS orders_id, "
+ "orders.address_id AS orders_address_id, "
+ "orders.description AS orders_description, "
+ "orders.isopen AS orders_isopen, "
+ "items_1.id AS items_1_id, "
+ "items_1.description AS items_1_description "
+ "FROM orders LEFT OUTER JOIN "
+ "(order_items AS order_items_1 "
+ "JOIN items AS items_1 "
+ "ON items_1.id = order_items_1.item_id "
+ "AND items_1.description = :description_1) "
+ "ON orders.id = order_items_1.order_id "
+ "WHERE orders.user_id IN ([POSTCOMPILE_primary_keys]) "
+ "AND orders.description = :description_2 "
+ "ORDER BY orders.id, items_1.id",
+ [
+ {
+ "description_1": item_description,
+ "primary_keys": [7],
+ "description_2": order_description,
+ }
+ ],
+ ),
+ )
+
def test_lazyload_local_criteria(self, user_address_fixture):
User, Address = user_address_fixture
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
