summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/build/changelog/unreleased_14/6459.rst10
-rw-r--r--lib/sqlalchemy/orm/attributes.py6
-rw-r--r--lib/sqlalchemy/orm/context.py4
-rw-r--r--lib/sqlalchemy/orm/util.py2
-rw-r--r--lib/sqlalchemy/sql/base.py9
-rw-r--r--lib/sqlalchemy/sql/traversals.py18
-rw-r--r--lib/sqlalchemy/sql/visitors.py2
-rw-r--r--test/orm/test_cache_key.py64
-rw-r--r--test/orm/test_subquery_relations.py45
-rw-r--r--test/orm/test_utils.py15
10 files changed, 162 insertions, 13 deletions
diff --git a/doc/build/changelog/unreleased_14/6459.rst b/doc/build/changelog/unreleased_14/6459.rst
new file mode 100644
index 000000000..dd5e84a93
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6459.rst
@@ -0,0 +1,10 @@
+.. change::
+ :tags: bug, orm
+ :tickets: 6459
+
+ Fixed issue in subquery loader strategy which prevented caching from
+ working correctly. This would have been seen in the logs as a "generated"
+ message instead of "cached" for all subqueryload SQL emitted, which by
+ saturating the cache with new keys would degrade overall performance; it
+ also would produce "LRU size alert" warnings.
+
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 2f8c8f940..05b12dda2 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -227,7 +227,7 @@ class QueryableAttribute(
else:
annotations = {
"proxy_key": self.key,
- "proxy_owner": self.class_,
+ "proxy_owner": self._parententity,
"entity_namespace": self._entity_namespace,
}
@@ -533,6 +533,10 @@ def create_proxied_attribute(descriptor):
)
@property
+ def _parententity(self):
+ return inspection.inspect(self.class_, raiseerr=False)
+
+ @property
def _entity_namespace(self):
if hasattr(self._comparator, "_parententity"):
return self._comparator._parententity
diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index aeba9ed80..ea84805b4 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -2692,9 +2692,9 @@ class _ORMColumnEntity(_ColumnEntity):
# within internal loaders.
orm_key = annotations.get("proxy_key", None)
- proxy_owner = annotations.get("proxy_owner", _entity.entity)
+ proxy_owner = annotations.get("proxy_owner", _entity)
if orm_key:
- self.expr = getattr(proxy_owner, orm_key)
+ self.expr = getattr(proxy_owner.entity, orm_key)
self.translate_raw_column = False
else:
# if orm_key is not present, that means this is an ad-hoc
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 65fdcf9fc..856afabb0 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -723,7 +723,6 @@ class AliasedInsp(
"parentmapper": self.mapper,
"parententity": self,
"entity_namespace": self,
- "compile_state_plugin": "orm",
}
)._set_propagate_attrs(
{"compile_state_plugin": "orm", "plugin_subject": self}
@@ -784,7 +783,6 @@ class AliasedInsp(
d = {
"parententity": self,
"parentmapper": self.mapper,
- "compile_state_plugin": "orm",
}
if key:
d["proxy_key"] = key
diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py
index 6d65d9061..8ff907ef0 100644
--- a/lib/sqlalchemy/sql/base.py
+++ b/lib/sqlalchemy/sql/base.py
@@ -781,7 +781,10 @@ class Executable(roles.StatementRole, Generative):
_executable_traverse_internals = [
("_with_options", InternalTraversal.dp_executable_options),
- ("_with_context_options", ExtendedInternalTraversal.dp_plain_obj),
+ (
+ "_with_context_options",
+ ExtendedInternalTraversal.dp_with_context_options,
+ ),
("_propagate_attrs", ExtendedInternalTraversal.dp_propagate_attrs),
]
@@ -853,8 +856,8 @@ class Executable(roles.StatementRole, Generative):
These are callable functions that will
be given the CompileState object upon compilation.
- A second argument cache_args is required, which will be combined
- with the identity of the function itself in order to produce a
+ A second argument cache_args is required, which will be combined with
+ the ``__code__`` identity of the function itself in order to produce a
cache key.
"""
diff --git a/lib/sqlalchemy/sql/traversals.py b/lib/sqlalchemy/sql/traversals.py
index e8a805285..e64eff6a4 100644
--- a/lib/sqlalchemy/sql/traversals.py
+++ b/lib/sqlalchemy/sql/traversals.py
@@ -310,6 +310,12 @@ class CacheKey(namedtuple("CacheKey", ["key", "bindparams"])):
def __eq__(self, other):
return self.key == other.key
+ @classmethod
+ def _diff_tuples(cls, left, right):
+ ck1 = CacheKey(left, [])
+ ck2 = CacheKey(right, [])
+ return ck1._diff(ck2)
+
def _whats_different(self, other):
k1 = self.key
@@ -413,6 +419,11 @@ class _CacheKey(ExtendedInternalTraversal):
visit_propagate_attrs = PROPAGATE_ATTRS
+ def visit_with_context_options(
+ self, attrname, obj, parent, anon_map, bindparams
+ ):
+ return tuple((fn.__code__, c_key) for fn, c_key in obj)
+
def visit_inspectable(self, attrname, obj, parent, anon_map, bindparams):
return (attrname, inspect(obj)._gen_cache_key(anon_map, bindparams))
@@ -1209,6 +1220,13 @@ class TraversalComparatorStrategy(InternalTraversal, util.MemoizedSlots):
else:
return left == right
+ def visit_with_context_options(
+ self, attrname, left_parent, left, right_parent, right, **kw
+ ):
+ return tuple((fn.__code__, c_key) for fn, c_key in left) == tuple(
+ (fn.__code__, c_key) for fn, c_key in right
+ )
+
def visit_plain_obj(
self, attrname, left_parent, left, right_parent, right, **kw
):
diff --git a/lib/sqlalchemy/sql/visitors.py b/lib/sqlalchemy/sql/visitors.py
index 5d60774aa..793e7fc5c 100644
--- a/lib/sqlalchemy/sql/visitors.py
+++ b/lib/sqlalchemy/sql/visitors.py
@@ -272,6 +272,8 @@ class InternalTraversal(util.with_metaclass(_InternalTraversalType, object)):
dp_executable_options = symbol("EO")
+ dp_with_context_options = symbol("WC")
+
dp_fromclause_ordered_set = symbol("CO")
"""Visit an ordered set of :class:`_expression.FromClause` objects. """
diff --git a/test/orm/test_cache_key.py b/test/orm/test_cache_key.py
index d120b05c0..390fc514d 100644
--- a/test/orm/test_cache_key.py
+++ b/test/orm/test_cache_key.py
@@ -1,4 +1,5 @@
import random
+import types
from sqlalchemy import func
from sqlalchemy import inspect
@@ -12,6 +13,7 @@ from sqlalchemy.orm import defaultload
from sqlalchemy.orm import defer
from sqlalchemy.orm import join as orm_join
from sqlalchemy.orm import joinedload
+from sqlalchemy.orm import lazyload
from sqlalchemy.orm import Load
from sqlalchemy.orm import mapper
from sqlalchemy.orm import Query
@@ -27,6 +29,7 @@ from sqlalchemy.sql.expression import case
from sqlalchemy.sql.visitors import InternalTraversal
from sqlalchemy.testing import AssertsCompiledSQL
from sqlalchemy.testing import eq_
+from sqlalchemy.testing import mock
from sqlalchemy.testing.fixtures import fixture_session
from test.orm import _fixtures
from .inheritance import _poly_fixtures
@@ -562,7 +565,9 @@ class RoundTripTest(QueryTest, AssertsCompiledSQL):
User,
users,
properties={
- "addresses": relationship(Address, back_populates="user")
+ "addresses": relationship(
+ Address, back_populates="user", order_by=addresses.c.id
+ )
},
)
@@ -683,3 +688,60 @@ class RoundTripTest(QueryTest, AssertsCompiledSQL):
go()
else:
go()
+
+ @testing.combinations(
+ (lazyload, 2, 6),
+ (joinedload, 1, 0),
+ (selectinload, 2, 5),
+ (subqueryload, 2, 0),
+ argnames="strat,expected_stmt_cache,expected_lambda_cache",
+ )
+ def test_cache_key_loader_strategies(
+ self,
+ plain_fixture,
+ strat,
+ expected_stmt_cache,
+ expected_lambda_cache,
+ connection,
+ ):
+ User, Address = plain_fixture
+
+ cache = {}
+
+ connection = connection.execution_options(compiled_cache=cache)
+ sess = Session(connection)
+
+ with mock.patch(
+ "sqlalchemy.orm.strategies.LazyLoader._query_cache", cache
+ ), mock.patch(
+ "sqlalchemy.orm.strategies.SelectInLoader._query_cache", cache
+ ):
+
+ def go():
+ stmt = (
+ select(User)
+ .where(User.id == 7)
+ .options(strat(User.addresses))
+ )
+
+ u1 = sess.execute(stmt).scalars().first()
+ eq_(u1.addresses, [Address(id=1)])
+
+ go()
+
+ lc = len(cache)
+
+ stmt_entries = [
+ k for k in cache if not isinstance(k[0], types.CodeType)
+ ]
+ lambda_entries = [
+ k for k in cache if isinstance(k[0], types.CodeType)
+ ]
+
+ eq_(len(stmt_entries), expected_stmt_cache)
+ eq_(len(lambda_entries), expected_lambda_cache)
+
+ for i in range(3):
+ go()
+
+ eq_(len(cache), lc)
diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index d5f5f98b1..96106b49d 100644
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -89,6 +89,47 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
self.assert_sql_count(testing.db, go, 2)
+ def test_query_is_cached(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),
+ lazy="subquery",
+ order_by=Address.id,
+ )
+ },
+ )
+ query_cache = {}
+ sess = fixture_session()
+
+ def go():
+ sess.close()
+
+ stmt = select(User).filter(User.id == 7)
+
+ sess.execute(
+ stmt, execution_options={"compiled_cache": query_cache}
+ ).one()
+
+ for i in range(3):
+ go()
+
+ qclen = len(query_cache)
+
+ for i in range(5):
+ go()
+
+ eq_(len(query_cache), qclen)
+
def test_params_arent_cached(self):
users, Address, addresses, User = (
self.tables.users,
@@ -113,14 +154,14 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
u1 = (
sess.query(User)
- .execution_options(query_cache=query_cache)
+ .execution_options(compiled_cache=query_cache)
.filter(User.id == 7)
.one()
)
u2 = (
sess.query(User)
- .execution_options(query_cache=query_cache)
+ .execution_options(compiled_cache=query_cache)
.filter(User.id == 8)
.one()
)
diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py
index c0829e9b3..1a7fa636c 100644
--- a/test/orm/test_utils.py
+++ b/test/orm/test_utils.py
@@ -246,7 +246,7 @@ class AliasedClassTest(fixtures.TestBase, AssertsCompiledSQL):
"parententity": point_mapper,
"parentmapper": point_mapper,
"proxy_key": "x_alone",
- "proxy_owner": Point,
+ "proxy_owner": point_mapper,
},
)
eq_(
@@ -256,7 +256,7 @@ class AliasedClassTest(fixtures.TestBase, AssertsCompiledSQL):
"parententity": point_mapper,
"parentmapper": point_mapper,
"proxy_key": "x",
- "proxy_owner": Point,
+ "proxy_owner": point_mapper,
},
)
@@ -265,6 +265,17 @@ class AliasedClassTest(fixtures.TestBase, AssertsCompiledSQL):
a2 = aliased(Point)
eq_(str(a2.x_alone == alias.x), "point_1.x = point_2.x")
+ eq_(
+ a2.x._annotations,
+ {
+ "entity_namespace": inspect(a2),
+ "parententity": inspect(a2),
+ "parentmapper": point_mapper,
+ "proxy_key": "x",
+ "proxy_owner": inspect(a2),
+ },
+ )
+
sess = fixture_session()
self.assert_compile(
View Lorry Controller Status