diff options
| -rw-r--r-- | doc/build/changelog/unreleased_14/5761.rst | 10 | ||||
| -rw-r--r-- | doc/build/changelog/unreleased_14/5762.rst | 10 | ||||
| -rw-r--r-- | doc/build/orm/session_events.rst | 23 | ||||
| -rw-r--r-- | examples/extending_query/filter_public.py | 3 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/interfaces.py | 4 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/session.py | 41 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/util.py | 42 | ||||
| -rw-r--r-- | test/orm/test_events.py | 93 | ||||
| -rw-r--r-- | test/orm/test_relationship_criteria.py | 48 |
9 files changed, 258 insertions, 16 deletions
diff --git a/doc/build/changelog/unreleased_14/5761.rst b/doc/build/changelog/unreleased_14/5761.rst new file mode 100644 index 000000000..9e36f2a89 --- /dev/null +++ b/doc/build/changelog/unreleased_14/5761.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + :tickets: 5761 + + Added new attribute :attr:`_orm.ORMExecuteState.is_column_load` to indicate + that a :meth:`_orm.SessionEvents.do_orm_execute` handler that a particular + operation is a primary-key-directed column attribute load, such as from an + expiration or a deferred attribute, and that WHERE criteria or additional + loader options should not be added to the query. This has been added to + the examples which illustrate the :func:`_orm.with_loader_criteria` option.
\ No newline at end of file diff --git a/doc/build/changelog/unreleased_14/5762.rst b/doc/build/changelog/unreleased_14/5762.rst new file mode 100644 index 000000000..7b5a90cdf --- /dev/null +++ b/doc/build/changelog/unreleased_14/5762.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + :tickets: 5762 + + The :func:`_orm.with_loader_criteria` option has been modified so that it + will never apply its criteria to the SELECT statement for an ORM refresh + operation, such as that invoked by :meth:`_orm.Session.refresh` or whenever + an expired attribute is loaded. These queries are only against the + primary key row of the object that is already present in memory so there + should not be additional criteria added.
\ No newline at end of file diff --git a/doc/build/orm/session_events.rst b/doc/build/orm/session_events.rst index 0040f6f47..544a6c577 100644 --- a/doc/build/orm/session_events.rst +++ b/doc/build/orm/session_events.rst @@ -87,7 +87,12 @@ may be used on its own, or is ideally suited to be used within the @event.listens_for(Session, "do_orm_execute") def _do_orm_execute(orm_execute_state): - if orm_execute_state.is_select: + + if ( + orm_execute_state.is_select and + not orm_execute_state.is_column_load and + not orm_execute_state.is_relationship_load + ): orm_execute_state.statement = orm_execute_state.statement.options( with_loader_criteria(MyEntity.public == True) ) @@ -95,7 +100,9 @@ may be used on its own, or is ideally suited to be used within the Above, an option is added to all SELECT statements that will limit all queries against ``MyEntity`` to filter on ``public == True``. The criteria will be applied to **all** loads of that class within the scope of the -immediate query as well as subsequent relationship loads, which includes +immediate query. The :func:`_orm.with_loader_criteria` option by default +will automatically propagate to relationship loaders as well, which will +apply to subsequent relationship loads, which includes lazy loads, selectinloads, etc. For a series of classes that all feature some common column structure, @@ -127,7 +134,11 @@ to intercept all objects that extend from ``HasTimestamp`` and filter their @event.listens_for(Session, "do_orm_execute") def _do_orm_execute(orm_execute_state): - if orm_execute_state.is_select: + if ( + orm_execute_state.is_select + and not orm_execute_state.is_column_load + and not orm_execute_state.is_relationship_load + ): one_month_ago = datetime.datetime.today() - datetime.timedelta(months=1) orm_execute_state.statement = orm_execute_state.statement.options( @@ -138,6 +149,12 @@ to intercept all objects that extend from ``HasTimestamp`` and filter their ) ) +.. warning:: The use of a lambda inside of the call to + :func:`_orm.with_loader_criteria` is only invoked **once per unique class**. + Custom functions should not be invoked within this lambda. See + :ref:`engine_lambda_caching` for an overview of the "lambda SQL" feature, + which is for advanced use only. + .. seealso:: :ref:`examples_session_orm_events` - includes working examples of the diff --git a/examples/extending_query/filter_public.py b/examples/extending_query/filter_public.py index b6d4ec0db..53472e95a 100644 --- a/examples/extending_query/filter_public.py +++ b/examples/extending_query/filter_public.py @@ -37,7 +37,8 @@ def _add_filtering_criteria(execute_state): # query. if ( - not execute_state.is_relationship_load + not execute_state.is_column_load + and not execute_state.is_relationship_load and not execute_state.execution_options.get("include_private", False) ): execute_state.statement = execute_state.statement.options( diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index 64f561cbd..bacec422c 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -724,8 +724,8 @@ class ORMOption(ExecutableOption): propagate_to_loaders = False """if True, indicate this option should be carried along - to "secondary" Query objects produced during lazy loads - or refresh operations. + to "secondary" SELECT statements that occur for relationship + lazy loaders as well as attribute load / refresh operations. """ diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 2a531b118..f6943cc5f 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -272,6 +272,8 @@ class ORMExecuteState(util.MemoizedSlots): self.local_execution_options = self.local_execution_options.union(opts) def _orm_compile_options(self): + if not self.is_select: + return None opts = self.statement._compile_options if isinstance(opts, context.ORMCompileState.default_compile_options): return opts @@ -308,6 +310,33 @@ class ORMExecuteState(util.MemoizedSlots): return None @property + def is_column_load(self): + """Return True if the operation is refreshing column-oriented + attributes on an existing ORM object. + + This occurs during operations such as :meth:`_orm.Session.refresh`, + as well as when an attribute deferred by :func:`_orm.defer` is + being loaded, or an attribute that was expired either directly + by :meth:`_orm.Session.expire` or via a commit operation is being + loaded. + + Handlers will very likely not want to add any options to queries + when such an operation is occurring as the query should be a straight + primary key fetch which should not have any additional WHERE criteria, + and loader options travelling with the instance + will have already been added to the query. + + .. versionadded:: 1.4.0b2 + + .. seealso:: + + :attr:`_orm.ORMExecuteState.is_relationship_load` + + """ + opts = self._orm_compile_options() + return opts is not None and opts._for_refresh_state + + @property def is_relationship_load(self): """Return True if this load is loading objects on behalf of a relationship. @@ -317,7 +346,19 @@ class ORMExecuteState(util.MemoizedSlots): SELECT statement being emitted is on behalf of a relationship load. + Handlers will very likely not want to add any options to queries + when such an operation is occurring, as loader options are already + capable of being propigated to relationship loaders and should + be already present. + + .. seealso:: + + :attr:`_orm.ORMExecuteState.is_column_load` + """ + opts = self._orm_compile_options() + if opts is None: + return False path = self.loader_strategy_path return path is not None and not path.is_root diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 886ae9a11..c9437d1b2 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -935,17 +935,38 @@ class LoaderCriteriaOption(CriteriaOption): @event.listens_for("do_orm_execute", session) def _add_filtering_criteria(execute_state): - execute_state.statement = execute_state.statement.options( - with_loader_criteria( - SecurityRole, - lambda cls: cls.role.in_(['some_role']), - include_aliases=True - ) - ) - The given class will expand to include all mapped subclass and - need not itself be a mapped class. + if ( + execute_state.is_select + and not execute_state.is_column_load + and not execute_state.is_relationship_load + ): + execute_state.statement = execute_state.statement.options( + with_loader_criteria( + SecurityRole, + lambda cls: cls.role.in_(['some_role']), + include_aliases=True + ) + ) + In the above example, the :meth:`_orm.SessionEvents.do_orm_execute` + event will intercept all queries emitted using the + :class:`_orm.Session`. For those queries which are SELECT statements + and are not attribute or relationship loads a custom + :func:`_orm.with_loader_criteria` option is added to the query. The + :func:`_orm.with_loader_criteria` option will be used in the given + statement and will also be automatically propagated to all relationship + loads that descend from this query. + + The criteria argument given is a ``lambda`` that accepts a ``cls`` + argument. The given class will expand to include all mapped subclass + and need not itself be a mapped class. + + .. warning:: The use of a lambda inside of the call to + :func:`_orm.with_loader_criteria` is only invoked **once per unique + class**. Custom functions should not be invoked within this lambda. + See :ref:`engine_lambda_caching` for an overview of the "lambda SQL" + feature, which is for advanced use only. :param entity_or_base: a mapped class, or a class that is a super class of a particular set of mapped classes, to which the rule @@ -1028,7 +1049,8 @@ class LoaderCriteriaOption(CriteriaOption): # if options to limit the criteria to immediate query only, # use compile_state.attributes instead - self.get_global_criteria(compile_state.global_attributes) + if not compile_state.compile_options._for_refresh_state: + self.get_global_criteria(compile_state.global_attributes) def get_global_criteria(self, attributes): for mp in self._all_mappers(): diff --git a/test/orm/test_events.py b/test/orm/test_events.py index f8600894f..bc72d2f21 100644 --- a/test/orm/test_events.py +++ b/test/orm/test_events.py @@ -1,4 +1,5 @@ import sqlalchemy as sa +from sqlalchemy import delete from sqlalchemy import event from sqlalchemy import ForeignKey from sqlalchemy import Integer @@ -6,6 +7,7 @@ from sqlalchemy import literal_column from sqlalchemy import select from sqlalchemy import String from sqlalchemy import testing +from sqlalchemy import update from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import attributes from sqlalchemy.orm import class_mapper @@ -166,6 +168,97 @@ class ORMExecuteTest(_RemoveListeners, _fixtures.FixtureTest): }, ) + def test_flags(self): + User, Address = self.classes("User", "Address") + + sess = Session(testing.db, future=True) + + canary = Mock() + + @event.listens_for(sess, "do_orm_execute") + def do_orm_execute(ctx): + + if not ctx.is_select: + assert_raises_message( + sa.exc.InvalidRequestError, + "This ORM execution is not against a SELECT statement", + lambda: ctx.lazy_loaded_from, + ) + + canary.options( + is_select=ctx.is_select, + is_update=ctx.is_update, + is_delete=ctx.is_delete, + is_orm_statement=ctx.is_orm_statement, + is_relationship_load=ctx.is_relationship_load, + is_column_load=ctx.is_column_load, + lazy_loaded_from=ctx.lazy_loaded_from + if ctx.is_select + else None, + ) + + u1 = sess.execute(select(User).filter_by(id=7)).scalar_one() + + u1.addresses + + sess.expire(u1) + + eq_(u1.name, "jack") + + sess.execute(delete(User).filter_by(id=18)) + sess.execute(update(User).filter_by(id=18).values(name="eighteen")) + + eq_( + canary.mock_calls, + [ + call.options( + is_select=True, + is_update=False, + is_delete=False, + is_orm_statement=True, + is_relationship_load=False, + is_column_load=False, + lazy_loaded_from=None, + ), + call.options( + is_select=True, + is_update=False, + is_delete=False, + is_orm_statement=True, + is_relationship_load=False, + is_column_load=False, + lazy_loaded_from=u1._sa_instance_state, + ), + call.options( + is_select=True, + is_update=False, + is_delete=False, + is_orm_statement=True, + is_relationship_load=False, + is_column_load=True, + lazy_loaded_from=None, + ), + call.options( + is_select=False, + is_update=False, + is_delete=True, + is_orm_statement=True, + is_relationship_load=False, + is_column_load=False, + lazy_loaded_from=None, + ), + call.options( + is_select=False, + is_update=True, + is_delete=False, + is_orm_statement=True, + is_relationship_load=False, + is_column_load=False, + lazy_loaded_from=None, + ), + ], + ) + def test_chained_events_two(self): sess = Session(testing.db, future=True) diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py index 7237dd264..87589d3be 100644 --- a/test/orm/test_relationship_criteria.py +++ b/test/orm/test_relationship_criteria.py @@ -12,6 +12,7 @@ from sqlalchemy import sql from sqlalchemy import String from sqlalchemy import testing from sqlalchemy.orm import aliased +from sqlalchemy.orm import defer from sqlalchemy.orm import joinedload from sqlalchemy.orm import lazyload from sqlalchemy.orm import mapper @@ -597,6 +598,53 @@ class LoaderCriteriaTest(_Fixtures, testing.AssertsCompiledSQL): eq_(s.execute(stmt).scalars().all(), [UserWFoob(name=name)]) + def test_never_for_refresh(self, user_address_fixture): + User, Address = user_address_fixture + + s = Session(testing.db) + u1 = s.get(User, 8) + + @event.listens_for(s, "do_orm_execute") + def add_criteria(orm_context): + orm_context.statement = orm_context.statement.options( + with_loader_criteria(User, User.id != 8) + ) + + s.refresh(u1) + eq_(u1.name, "ed") + + def test_never_for_unexpire(self, user_address_fixture): + User, Address = user_address_fixture + + s = Session(testing.db) + u1 = s.get(User, 8) + + s.expire(u1) + + @event.listens_for(s, "do_orm_execute") + def add_criteria(orm_context): + orm_context.statement = orm_context.statement.options( + with_loader_criteria(User, User.id != 8) + ) + + eq_(u1.name, "ed") + + def test_never_for_undefer(self, user_address_fixture): + User, Address = user_address_fixture + + s = Session(testing.db) + u1 = s.execute( + select(User).options(defer(User.name)).filter(User.id == 8) + ).scalar_one() + + @event.listens_for(s, "do_orm_execute") + def add_criteria(orm_context): + orm_context.statement = orm_context.statement.options( + with_loader_criteria(User, User.id != 8) + ) + + eq_(u1.name, "ed") + class TemporalFixtureTest(testing.fixtures.DeclarativeMappedTest): @classmethod |