From: Mike Bayer Date: Sun, 13 Oct 2024 14:04:23 +0000 (-0400) Subject: consult allow_partial_pks for NULL check in lazyload X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=830debc30896203bfd21fea18d323c5d849068d1;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git consult allow_partial_pks for NULL check in lazyload Refined the check which the ORM lazy loader uses to detect "this would be loading by primary key and the primary key is NULL, skip loading" to take into account the current setting for the :paramref:`.orm.Mapper.allow_partial_pks` parameter. If this parameter is False, then a composite PK value that has partial NULL elements should also be skipped. This can apply to some composite overlapping foreign key configurations. Fixes: #11995 Change-Id: Icf9a52b7405d7400d46bfa944edcbff1a89225a3 --- diff --git a/doc/build/changelog/unreleased_20/11995.rst b/doc/build/changelog/unreleased_20/11995.rst new file mode 100644 index 0000000000..a748a1c5df --- /dev/null +++ b/doc/build/changelog/unreleased_20/11995.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, orm + :tickets: 11995 + + Refined the check which the ORM lazy loader uses to detect "this would be + loading by primary key and the primary key is NULL, skip loading" to take + into account the current setting for the + :paramref:`.orm.Mapper.allow_partial_pks` parameter. If this parameter is + False, then a composite PK value that has partial NULL elements should also + be skipped. This can apply to some composite overlapping foreign key + configurations. + diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py index c9005298d8..b5f7dbbafb 100644 --- a/lib/sqlalchemy/orm/base.py +++ b/lib/sqlalchemy/orm/base.py @@ -283,6 +283,8 @@ _never_set = frozenset([NEVER_SET]) _none_set = frozenset([None, NEVER_SET, PASSIVE_NO_RESULT]) +_none_only_set = frozenset([None]) + _SET_DEFERRED_EXPIRED = util.symbol("SET_DEFERRED_EXPIRED") _DEFER_FOR_STATE = util.symbol("DEFER_FOR_STATE") diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 59c8d01145..b15c6e0513 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -298,6 +298,17 @@ class Mapper( particular primary key value. A "partial primary key" can occur if one has mapped to an OUTER JOIN, for example. + The :paramref:`.orm.Mapper.allow_partial_pks` parameter also + indicates to the ORM relationship lazy loader, when loading a + many-to-one related object, if a composite primary key that has + partial NULL values should result in an attempt to load from the + database, or if a load attempt is not necessary. + + .. versionadded:: 2.0.36 :paramref:`.orm.Mapper.allow_partial_pks` + is consulted by the relationship lazy loader strategy, such that + when set to False, a SELECT for a composite primary key that + has partial NULL values will not be emitted. + :param batch: Defaults to ``True``, indicating that save operations of multiple entities can be batched together for efficiency. Setting to False indicates diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 3f947a8d74..c89a12efd6 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -47,7 +47,7 @@ from .interfaces import StrategizedProperty from .session import _state_session from .state import InstanceState from .strategy_options import Load -from .util import _none_set +from .util import _none_only_set from .util import AliasedClass from .. import event from .. import exc as sa_exc @@ -936,8 +936,15 @@ class LazyLoader( elif LoaderCallableStatus.NEVER_SET in primary_key_identity: return LoaderCallableStatus.NEVER_SET - if _none_set.issuperset(primary_key_identity): - return None + # test for None alone in primary_key_identity based on + # allow_partial_pks preference. PASSIVE_NO_RESULT and NEVER_SET + # have already been tested above + if not self.mapper.allow_partial_pks: + if _none_only_set.intersection(primary_key_identity): + return None + else: + if _none_only_set.issuperset(primary_key_identity): + return None if ( self.key in state.dict diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 2a1f4bfe4c..0360eb20e8 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -43,6 +43,7 @@ from ._typing import prop_is_relationship from .base import _class_to_mapper as _class_to_mapper from .base import _MappedAnnotationBase from .base import _never_set as _never_set # noqa: F401 +from .base import _none_only_set as _none_only_set # noqa: F401 from .base import _none_set as _none_set # noqa: F401 from .base import attribute_str as attribute_str # noqa: F401 from .base import class_mapper as class_mapper diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py index 64c86853d2..9bb8071984 100644 --- a/test/orm/test_lazy_relations.py +++ b/test/orm/test_lazy_relations.py @@ -21,7 +21,9 @@ from sqlalchemy.orm import aliased from sqlalchemy.orm import attributes from sqlalchemy.orm import configure_mappers from sqlalchemy.orm import exc as orm_exc +from sqlalchemy.orm import foreign from sqlalchemy.orm import relationship +from sqlalchemy.orm import remote from sqlalchemy.orm import Session from sqlalchemy.orm import with_parent from sqlalchemy.testing import assert_raises @@ -1270,6 +1272,54 @@ class M2OGetTest(_fixtures.FixtureTest): self.assert_sql_count(testing.db, go, 1) + @testing.fixture() + def composite_overlapping_fixture(self, decl_base, connection): + def go(allow_partial_pks): + + class Section(decl_base): + __tablename__ = "sections" + year = Column(Integer, primary_key=True) + idx = Column(Integer, primary_key=True) + parent_idx = Column(Integer) + + if not allow_partial_pks: + __mapper_args__ = {"allow_partial_pks": False} + + ForeignKeyConstraint((year, parent_idx), (year, idx)) + + parent = relationship( + "Section", + primaryjoin=and_( + year == remote(year), + foreign(parent_idx) == remote(idx), + ), + ) + + decl_base.metadata.create_all(connection) + connection.commit() + + with Session(connection) as sess: + sess.add(Section(year=5, idx=1, parent_idx=None)) + sess.commit() + + return Section + + return go + + @testing.variation("allow_partial_pks", [True, False]) + def test_composite_m2o_load_partial_pks( + self, allow_partial_pks, composite_overlapping_fixture + ): + Section = composite_overlapping_fixture(allow_partial_pks) + + session = fixture_session() + section = session.get(Section, (5, 1)) + + with self.assert_statement_count( + testing.db, 1 if allow_partial_pks else 0 + ): + testing.is_none(section.parent) + class CorrelatedTest(fixtures.MappedTest): @classmethod