From 5ef1b89d865679fa2ca4bb3e3c1892bdd966ad89 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 6 Apr 2021 16:47:00 -0400 Subject: Disable and disallow Result.unique() with yield_per Fixed critical regression where the :meth:`_orm.Query.yield_per` method in the ORM would set up the internal :class:`_engine.Result` to yield chunks at a time, however made use of the new :meth:`_engine.Result.unique` method which uniques across the entire result. This would lead to lost rows since the ORM is using ``id(obj)`` as the uniquing function, which leads to repeated identifiers for new objects as already-seen objects are garbage collected. 1.3's behavior here was to "unique" across each chunk, which does not actually produce "uniqued" results when results are yielded in chunks. As the :meth:`_orm.Query.yield_per` method is already explicitly disallowed when joined eager loading is in place, which is the primary rationale for the "uniquing" feature, the "uniquing" feature is now turned off entirely when :meth:`_orm.Query.yield_per` is used. This regression only applies to the legacy :class:`_orm.Query` object; when using :term:`2.0 style` execution, "uniquing" is not automatically applied. To prevent the issue from arising from explicit use of :meth:`_engine.Result.unique`, an error is now raised if rows are fetched from a "uniqued" ORM-level :class:`_engine.Result` if any :ref:`yield per ` API is also in use, as the purpose of ``yield_per`` is to allow for arbitrarily large numbers of rows, which cannot be uniqued in memory without growing the number of entries to fit the complete result size. Fixes: #6206 Change-Id: I3770d1f2e9be44d82c83ca992afb912dcc17af05 --- lib/sqlalchemy/orm/loading.py | 11 ++++++++++- lib/sqlalchemy/orm/query.py | 5 ++++- 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py index 4a90caeff..9dcaca0ea 100644 --- a/lib/sqlalchemy/orm/loading.py +++ b/lib/sqlalchemy/orm/loading.py @@ -87,11 +87,20 @@ def instances(cursor, context): with util.safe_reraise(): cursor.close() + def _no_unique(entry): + raise sa_exc.InvalidRequestError( + "Can't use the ORM yield_per feature in conjunction with unique()" + ) + row_metadata = SimpleResultMetaData( labels, extra, _unique_filters=[ - id if ent.use_id_for_hash else None + _no_unique + if context.yield_per + else id + if ent.use_id_for_hash + else None for ent in context.compile_state._entities ], ) diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 38f1d26b4..9348d7d48 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -2827,7 +2827,10 @@ class Query( if result._attributes.get("is_single_entity", False): result = result.scalars() - if result._attributes.get("filtered", False): + if ( + result._attributes.get("filtered", False) + and not self.load_options._yield_per + ): result = result.unique() return result -- cgit v1.2.1