diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2022-11-10 17:01:58 -0500 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2022-11-11 15:25:35 -0500 |
commit | 1d8833a9c1ada64cfc716109a8836b32cb8a9bd6 (patch) | |
tree | ae5d2f6adfb578c43603b0141c27fad520c70e4f /lib/sqlalchemy/sql | |
parent | e3a8d198917f4246365e09fa975d55c64082cd2e (diff) | |
download | sqlalchemy-1d8833a9c1ada64cfc716109a8836b32cb8a9bd6.tar.gz |
ensure anon_map is passed for most annotated traversals
We can cache the annotated cache key for Table, but
for selectables it's not safe, as it fails to pass the
anon_map along and creates many redudant structures in
observed test scenario. It is likely safe for a
Column that's mapped to a Table also, however this is
not implemented here. Will have to see if that part
needs adjusting.
Fixed critical memory issue identified in cache key generation, where for
very large and complex ORM statements that make use of lots of ORM aliases
with subqueries, cache key generation could produce excessively large keys
that were orders of magnitude bigger than the statement itself. Much thanks
to Rollo Konig Brock for their very patient, long term help in finally
identifying this issue.
Also within TypeEngine objects, when we generate elements
for instance variables, skip the None elements at least.
this also saves on tuple complexity.
Fixes: #8790
Change-Id: I448ddbfb45ae0a648815be8dad4faad7d1977427
Diffstat (limited to 'lib/sqlalchemy/sql')
-rw-r--r-- | lib/sqlalchemy/sql/annotation.py | 8 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/cache_key.py | 17 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/elements.py | 27 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/schema.py | 11 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/type_api.py | 4 |
5 files changed, 57 insertions, 10 deletions
diff --git a/lib/sqlalchemy/sql/annotation.py b/lib/sqlalchemy/sql/annotation.py index 262048bd1..43ca84abb 100644 --- a/lib/sqlalchemy/sql/annotation.py +++ b/lib/sqlalchemy/sql/annotation.py @@ -94,12 +94,18 @@ class SupportsAnnotations(ExternallyTraversible): @util.memoized_property def _annotations_cache_key(self) -> Tuple[Any, ...]: anon_map_ = anon_map() + + return self._gen_annotations_cache_key(anon_map_) + + def _gen_annotations_cache_key( + self, anon_map: anon_map + ) -> Tuple[Any, ...]: return ( "_annotations", tuple( ( key, - value._gen_cache_key(anon_map_, []) + value._gen_cache_key(anon_map, []) if isinstance(value, HasCacheKey) else value, ) diff --git a/lib/sqlalchemy/sql/cache_key.py b/lib/sqlalchemy/sql/cache_key.py index 88148285c..39d09d3ab 100644 --- a/lib/sqlalchemy/sql/cache_key.py +++ b/lib/sqlalchemy/sql/cache_key.py @@ -297,12 +297,17 @@ class HasCacheKey: else None, ) elif meth is InternalTraversal.dp_annotations_key: - # obj is here is the _annotations dict. however, we - # want to use the memoized cache key version of it. for - # Columns, this should be long lived. For select() - # statements, not so much, but they usually won't have - # annotations. - result += self._annotations_cache_key # type: ignore + # obj is here is the _annotations dict. Table uses + # a memoized version of it. however in other cases, + # we generate it given anon_map as we may be from a + # Join, Aliased, etc. + # see #8790 + + if self._gen_static_annotations_cache_key: # type: ignore # noqa: E501 + result += self._annotations_cache_key # type: ignore # noqa: E501 + else: + result += self._gen_annotations_cache_key(anon_map) # type: ignore # noqa: E501 + elif ( meth is InternalTraversal.dp_clauseelement_list or meth is InternalTraversal.dp_clauseelement_tuple diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 3b70e8d4e..6a5aa7db9 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -334,6 +334,7 @@ class ClauseElement( _is_column_element = False _is_keyed_column_element = False _is_table = False + _gen_static_annotations_cache_key = False _is_textual = False _is_from_clause = False _is_returns_rows = False @@ -3224,7 +3225,7 @@ class Cast(WrapsColumnExpression[_T]): _traverse_internals: _TraverseInternalsType = [ ("clause", InternalTraversal.dp_clauseelement), - ("typeclause", InternalTraversal.dp_clauseelement), + ("type", InternalTraversal.dp_type), ] clause: ColumnElement[Any] @@ -3631,7 +3632,20 @@ class BinaryExpression(OperatorExpression[_T]): ( "type", InternalTraversal.dp_type, - ), # affects JSON CAST operators + ), + ] + + _cache_key_traversal = [ + ("left", InternalTraversal.dp_clauseelement), + ("right", InternalTraversal.dp_clauseelement), + ("operator", InternalTraversal.dp_operator), + ("modifiers", InternalTraversal.dp_plain_dict), + # "type" affects JSON CAST operators, so while redundant in most cases, + # is needed for that one + ( + "type", + InternalTraversal.dp_type, + ), ] _is_implicitly_boolean = True @@ -3816,6 +3830,10 @@ class Grouping(GroupedElement, ColumnElement[_T]): ("type", InternalTraversal.dp_type), ] + _cache_key_traversal = [ + ("element", InternalTraversal.dp_clauseelement), + ] + element: Union[TextClause, ClauseList, ColumnElement[_T]] def __init__( @@ -4322,6 +4340,11 @@ class Label(roles.LabeledColumnExprRole[_T], NamedColumn[_T]): ("_element", InternalTraversal.dp_clauseelement), ] + _cache_key_traversal = [ + ("name", InternalTraversal.dp_anon_name), + ("_element", InternalTraversal.dp_clauseelement), + ] + _element: ColumnElement[_T] name: str diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py index 36c33868a..dde5cd372 100644 --- a/lib/sqlalchemy/sql/schema.py +++ b/lib/sqlalchemy/sql/schema.py @@ -2023,6 +2023,17 @@ class Column(DialectKWArgs, SchemaItem, ColumnClause[_T]): identity: Optional[Identity] + @util.memoized_property + def _gen_static_annotations_cache_key(self) -> bool: # type: ignore + """special attribute used by cache key gen, if true, we will + use a static cache key for the annotations dictionary, else we + will generate a new cache key for annotations each time. + + Added for #8790 + + """ + return self.table is not None and self.table._is_table + def _extra_kwargs(self, **kwargs: Any) -> None: self._validate_dialect_kwargs(kwargs) diff --git a/lib/sqlalchemy/sql/type_api.py b/lib/sqlalchemy/sql/type_api.py index 90320701e..cd57ee3b6 100644 --- a/lib/sqlalchemy/sql/type_api.py +++ b/lib/sqlalchemy/sql/type_api.py @@ -940,7 +940,9 @@ class TypeEngine(Visitable, Generic[_T]): else self.__dict__[k], ) for k in names - if k in self.__dict__ and not k.startswith("_") + if k in self.__dict__ + and not k.startswith("_") + and self.__dict__[k] is not None ) @overload |