:ticket:`12168`
+.. _change_12570:
+
+New rules for None-return for ORM Composites
+--------------------------------------------
+
+ORM composite attributes configured using :func:`_orm.composite` can now
+specify whether or not they should return ``None`` using a new parameter
+:paramref:`_orm.composite.return_none_on`. By default, a composite
+attribute now returns a non-None object in all cases, whereas previously
+under 2.0, a ``None`` value would be returned for a pending object with
+``None`` values for all composite columns.
+
+Given a composite mapping::
+
+ import dataclasses
+
+
+ @dataclasses.dataclass
+ class Point:
+ x: int | None
+ y: int | None
+
+
+ class Base(DeclarativeBase):
+ pass
+
+
+ class Vertex(Base):
+ __tablename__ = "vertices"
+
+ id: Mapped[int] = mapped_column(primary_key=True)
+
+ start: Mapped[Point] = composite(mapped_column("x1"), mapped_column("y1"))
+ end: Mapped[Point] = composite(mapped_column("x2"), mapped_column("y2"))
+
+When constructing a pending ``Vertex`` object, the initial value of the
+``x1``, ``y1``, ``x2``, ``y2`` columns is ``None``. Under version 2.0,
+accessing the composite at this stage would automatically return ``None``::
+
+ >>> v1 = Vertex()
+ >>> v1.start
+ None
+
+Under 2.1, the default behavior is to return the composite class with attributes
+set to ``None``::
+
+ >>> v1 = Vertex()
+ >>> v1.start
+ Point(x=None, y=None)
+
+This behavior is now consistent with other forms of access, such as accessing
+the attribute from a persistent object as well as querying for the attribute
+directly. It is also consistent with the mapped annotation ``Mapped[Point]``.
+
+The behavior can be further controlled by applying the
+:paramref:`_orm.composite.return_none_on` parameter, which accepts a callable
+that returns True if the composite should be returned as None, given the
+arguments that would normally be passed to the composite class. The typical callable
+here would return True (i.e. the value should be ``None``) for the case where all
+columns are ``None``::
+
+ class Vertex(Base):
+ __tablename__ = "vertices"
+
+ id: Mapped[int] = mapped_column(primary_key=True)
+
+ start: Mapped[Point] = composite(
+ mapped_column("x1"),
+ mapped_column("y1"),
+ return_none_on=lambda x, y: x is None and y is None,
+ )
+ end: Mapped[Point] = composite(
+ mapped_column("x2"),
+ mapped_column("y2"),
+ return_none_on=lambda x, y: x is None and y is None,
+ )
+
+For the above class, any ``Vertex`` instance whether pending or persistent will
+return ``None`` for ``start`` and ``end`` if both composite columns for the attribute
+are ``None``::
+
+ >>> v1 = Vertex()
+ >>> v1.start
+ None
+
+The :paramref:`_orm.composite.return_none_on` parameter is also set
+automatically, if not otherwise set explicitly, when using
+:ref:`orm_declarative_mapped_column`; setting the left hand side to
+``Optional`` or ``| None`` will assign the above ``None``-handling callable::
+
+
+ class Vertex(Base):
+ __tablename__ = "vertices"
+
+ id: Mapped[int] = mapped_column(primary_key=True)
+
+ # will apply return_none_on=lambda *args: all(arg is None for arg in args)
+ start: Mapped[Point | None] = composite(mapped_column("x1"), mapped_column("y1"))
+ end: Mapped[Point | None] = composite(mapped_column("x2"), mapped_column("y2"))
+
+The above object will return ``None`` for ``start`` and ``end`` automatically
+if the columns are also None::
+
+ >>> session.scalars(
+ ... select(Vertex.start).where(Vertex.x1 == None, Vertex.y1 == None)
+ ... ).first()
+ None
+
+If :paramref:`_orm.composite.return_none_on` is set explicitly, that value will
+supersede the choice made by ORM Annotated Declarative. This includes that
+the parameter may be explicitly set to ``None`` which will disable the ORM
+Annotated Declarative setting from taking place.
+
+:ticket:`12570`
+
New Features and Improvements - Core
=====================================
--- /dev/null
+.. change::
+ :tags: feature, orm
+ :tickets: 12570
+
+ Added new parameter :paramref:`_orm.composite.return_none_on` to
+ :func:`_orm.composite`, which allows control over if and when this
+ composite attribute should resolve to ``None`` when queried or retrieved
+ from the object directly. By default, a composite object is always present
+ on the attribute, including for a pending object which is a behavioral
+ change since 2.0. When :paramref:`_orm.composite.return_none_on` is
+ specified, a callable is passed that returns True or False to indicate if
+ the given arguments indicate the composite should be returned as None. This
+ parameter may also be set automatically when ORM Annotated Declarative is
+ used; if the annotation is given as ``Mapped[SomeClass|None]``, a
+ :paramref:`_orm.composite.return_none_on` rule is applied that will return
+ ``None`` if all contained columns are themselves ``None``.
+
+ .. seealso::
+
+ :ref:`change_12570`
:ref:`mutable_toplevel` extension must be used. See the section
:ref:`mutable_composites` for examples.
+Returning None for a Composite
+-------------------------------
+
+The composite attribute by default always returns an object when accessed,
+regardless of the values of its columns. In the example below, a new
+``Vertex`` is created with no parameters; all column attributes ``x1``, ``y1``,
+``x2``, and ``y2`` start out as ``None``. A ``Point`` object with ``None``
+values will be returned on access::
+
+ >>> v1 = Vertex()
+ >>> v1.start
+ Point(x=None, y=None)
+ >>> v1.end
+ Point(x=None, y=None)
+
+This behavior is consistent with persistent objects and individual attribute
+queries as well::
+
+ >>> start = session.scalars(
+ ... select(Point.start).where(Point.x1 == None, Point.y1 == None)
+ ... ).first()
+ >>> start
+ Point(x=None, y=None)
+
+To support an optional ``Point`` field, we can make use
+of the :paramref:`_orm.composite.return_none_on` parameter, which allows
+the behavior to be customized with a lambda; this parameter is set automatically if we
+declare our composite fields as optional::
+
+ class Vertex(Base):
+ __tablename__ = "vertices"
+
+ id: Mapped[int] = mapped_column(primary_key=True)
+ start: Mapped[Point | None] = composite(mapped_column("x1"), mapped_column("y1"))
+ end: Mapped[Point | None] = composite(mapped_column("x2"), mapped_column("y2"))
+
+Above, the :paramref:`_orm.composite.return_none_on` parameter is set equivalently as::
+
+ composite(
+ mapped_column("x1"),
+ mapped_column("y1"),
+ return_none_on=lambda *args: all(arg is None for arg in args),
+ )
+
+With the above setting, a value of ``None`` is returned if the columns themselves
+are both ``None``::
+
+ >>> v1 = Vertex()
+ >>> v1.start
+ None
+
+ >>> start = session.scalars(
+ ... select(Point.start).where(Point.x1 == None, Point.y1 == None)
+ ... ).first()
+ >>> start
+ None
+
+.. versionchanged:: 2.1 - added the :paramref:`_orm.composite.return_none_on` parameter with
+ ORM Annotated Declarative support.
+
+ .. seealso::
+
+ :ref:`change_12570`
.. _orm_composite_other_forms:
group: Optional[str] = None,
deferred: bool = False,
raiseload: bool = False,
+ return_none_on: Union[_NoArg, None, Callable[..., bool]] = _NoArg.NO_ARG,
comparator_factory: Optional[Type[Composite.Comparator[_T]]] = None,
active_history: bool = False,
init: Union[_NoArg, bool] = _NoArg.NO_ARG,
group: Optional[str] = None,
deferred: bool = False,
raiseload: bool = False,
+ return_none_on: Union[_NoArg, None, Callable[..., bool]] = _NoArg.NO_ARG,
comparator_factory: Optional[Type[Composite.Comparator[_T]]] = None,
active_history: bool = False,
init: Union[_NoArg, bool] = _NoArg.NO_ARG,
group: Optional[str] = None,
deferred: bool = False,
raiseload: bool = False,
+ return_none_on: Union[_NoArg, None, Callable[..., bool]] = _NoArg.NO_ARG,
comparator_factory: Optional[Type[Composite.Comparator[_T]]] = None,
active_history: bool = False,
init: Union[_NoArg, bool] = _NoArg.NO_ARG,
group: Optional[str] = None,
deferred: bool = False,
raiseload: bool = False,
+ return_none_on: Union[_NoArg, None, Callable[..., bool]] = _NoArg.NO_ARG,
comparator_factory: Optional[Type[Composite.Comparator[_T]]] = None,
active_history: bool = False,
init: Union[_NoArg, bool] = _NoArg.NO_ARG,
scalar attribute should be loaded when replaced, if not
already loaded. See the same flag on :func:`.column_property`.
+ :param return_none_on=None: A callable that will be evaluated when the
+ composite object is to be constructed, which upon returning the boolean
+ value ``True`` will instead bypass the construction and cause the
+ resulting value to be None. This typically may be assigned a lambda
+ that will evaluate to True when all the columns within the composite
+ are themselves None, e.g.::
+
+ composite(
+ MyComposite, return_none_on=lambda *cols: all(x is None for x in cols)
+ )
+
+ The above lambda for :paramref:`.composite.return_none_on` is used
+ automatically when using ORM Annotated Declarative along with an optional
+ value within the :class:`.Mapped` annotation.
+
+ .. versionadded:: 2.1
+
:param group:
A group name for this property when marked as deferred.
.. versionadded:: 2.0.42
- """
+ """ # noqa: E501
+
if __kw:
raise _no_kw()
return Composite(
_class_or_attr,
*attrs,
+ return_none_on=return_none_on,
attribute_options=_AttributeOptions(
init,
repr,
from .interfaces import _MapsColumns
from .interfaces import MapperProperty
from .interfaces import PropComparator
-from .util import _none_set
from .util import de_stringify_annotation
from .. import event
from .. import exc as sa_exc
from ..sql import operators
from ..sql.base import _NoArg
from ..sql.elements import BindParameter
+from ..util.typing import de_optionalize_union_types
+from ..util.typing import includes_none
from ..util.typing import is_fwd_ref
from ..util.typing import is_pep593
+from ..util.typing import is_union
from ..util.typing import TupleAny
from ..util.typing import Unpack
None, Type[_CC], Callable[..., _CC], _CompositeAttrType[Any]
] = None,
*attrs: _CompositeAttrType[Any],
+ return_none_on: Union[
+ _NoArg, None, Callable[..., bool]
+ ] = _NoArg.NO_ARG,
attribute_options: Optional[_AttributeOptions] = None,
active_history: bool = False,
deferred: bool = False,
self.composite_class = _class_or_attr # type: ignore
self.attrs = attrs
+ self.return_none_on = return_none_on
self.active_history = active_history
self.deferred = deferred
self.group = group
self._create_descriptor()
self._init_accessor()
+ @util.memoized_property
+ def _construct_composite(self) -> Callable[..., Any]:
+ return_none_on = self.return_none_on
+ if callable(return_none_on):
+
+ def construct(*args: Any) -> Any:
+ if return_none_on(*args):
+ return None
+ else:
+ return self.composite_class(*args)
+
+ return construct
+ else:
+ return self.composite_class
+
def instrument_class(self, mapper: Mapper[Any]) -> None:
super().instrument_class(mapper)
self._setup_event_handlers()
getattr(instance, key) for key in self._attribute_keys
]
- # current expected behavior here is that the composite is
- # created on access if the object is persistent or if
- # col attributes have non-None. This would be better
- # if the composite were created unconditionally,
- # but that would be a behavioral change.
- if self.key not in dict_ and (
- state.key is not None or not _none_set.issuperset(values)
- ):
- dict_[self.key] = self.composite_class(*values)
+ if self.key not in dict_:
+ dict_[self.key] = self._construct_composite(*values)
state.manager.dispatch.refresh(
state, self._COMPOSITE_FGET, [self.key]
)
cls, argument, originating_module, include_generic=True
)
+ if is_union(argument) and includes_none(argument):
+ if self.return_none_on is _NoArg.NO_ARG:
+ self.return_none_on = lambda *args: all(
+ arg is None for arg in args
+ )
+ argument = de_optionalize_union_types(argument)
+
self.composite_class = argument
if is_dataclass(self.composite_class):
if k not in dict_:
return
- dict_[self.key] = self.composite_class(
+ dict_[self.key] = self._construct_composite(
*[state.dict[key] for key in self._attribute_keys]
)
if has_history:
return attributes.History(
- [self.composite_class(*added)],
+ [self._construct_composite(*added)],
(),
- [self.composite_class(*deleted)],
+ [self._construct_composite(*deleted)],
)
else:
- return attributes.History((), [self.composite_class(*added)], ())
+ return attributes.History(
+ (), [self._construct_composite(*added)], ()
+ )
def _comparator_factory(
self, mapper: Mapper[Any]
labels: Sequence[str],
) -> Callable[[Row[Unpack[TupleAny]]], Any]:
def proc(row: Row[Unpack[TupleAny]]) -> Any:
- return self.property.composite_class(
+ return self.property._construct_composite(
*[proc(row) for proc in procs]
)
# round trip!
eq_(u1.address, Address("123 anywhere street"))
+ @testing.variation("explicit_col", [True, False])
+ @testing.variation("use_dataclass", [True, False])
+ @testing.variation("disable_none_on", [True, False])
+ def test_optional_composite(
+ self, decl_base, explicit_col, use_dataclass, disable_none_on
+ ):
+ """test #12570"""
+
+ global Point
+
+ if use_dataclass:
+
+ @dataclasses.dataclass
+ class Point:
+ x: Optional[int]
+ y: Optional[int]
+
+ else:
+
+ class Point:
+ def __init__(self, x, y):
+ self.x = x
+ self.y = y
+
+ def __composite_values__(self):
+ return (self.x, self.y)
+
+ def __eq__(self, other):
+ return (
+ isinstance(other, Point)
+ and self.x == other.x
+ and self.y == other.y
+ )
+
+ class Edge(decl_base):
+ __tablename__ = "edge"
+ id: Mapped[int] = mapped_column(primary_key=True)
+
+ if disable_none_on:
+ if explicit_col or not use_dataclass:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1", Integer, nullable=True),
+ mapped_column("y1", Integer, nullable=True),
+ return_none_on=None,
+ )
+ else:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1"),
+ mapped_column("y1"),
+ return_none_on=None,
+ )
+ else:
+ if explicit_col or not use_dataclass:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1", Integer, nullable=True),
+ mapped_column("y1", Integer, nullable=True),
+ )
+ else:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1"), mapped_column("y1")
+ )
+
+ eq_(Edge.__table__.c.x1.type._type_affinity, Integer)
+ eq_(Edge.__table__.c.y1.type._type_affinity, Integer)
+ is_true(Edge.__table__.c.x1.nullable)
+ is_true(Edge.__table__.c.y1.nullable)
+
+ decl_base.metadata.create_all(testing.db)
+
+ with Session(testing.db) as sess:
+ sess.add(Edge(start=None))
+ sess.commit()
+
+ if disable_none_on:
+ eq_(
+ sess.execute(select(Edge.start)).scalar_one(),
+ Point(x=None, y=None),
+ )
+ else:
+ eq_(
+ sess.execute(select(Edge.start)).scalar_one(),
+ None,
+ )
+
class AllYourFavoriteHitsTest(fixtures.TestBase, testing.AssertsCompiledSQL):
"""try a bunch of common mappings using the new style"""
# round trip!
eq_(u1.address, Address("123 anywhere street"))
+ @testing.variation("explicit_col", [True, False])
+ @testing.variation("use_dataclass", [True, False])
+ @testing.variation("disable_none_on", [True, False])
+ def test_optional_composite(
+ self, decl_base, explicit_col, use_dataclass, disable_none_on
+ ):
+ """test #12570"""
+
+ # anno only: global Point
+
+ if use_dataclass:
+
+ @dataclasses.dataclass
+ class Point:
+ x: Optional[int]
+ y: Optional[int]
+
+ else:
+
+ class Point:
+ def __init__(self, x, y):
+ self.x = x
+ self.y = y
+
+ def __composite_values__(self):
+ return (self.x, self.y)
+
+ def __eq__(self, other):
+ return (
+ isinstance(other, Point)
+ and self.x == other.x
+ and self.y == other.y
+ )
+
+ class Edge(decl_base):
+ __tablename__ = "edge"
+ id: Mapped[int] = mapped_column(primary_key=True)
+
+ if disable_none_on:
+ if explicit_col or not use_dataclass:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1", Integer, nullable=True),
+ mapped_column("y1", Integer, nullable=True),
+ return_none_on=None,
+ )
+ else:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1"),
+ mapped_column("y1"),
+ return_none_on=None,
+ )
+ else:
+ if explicit_col or not use_dataclass:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1", Integer, nullable=True),
+ mapped_column("y1", Integer, nullable=True),
+ )
+ else:
+ start: Mapped[Optional[Point]] = composite(
+ mapped_column("x1"), mapped_column("y1")
+ )
+
+ eq_(Edge.__table__.c.x1.type._type_affinity, Integer)
+ eq_(Edge.__table__.c.y1.type._type_affinity, Integer)
+ is_true(Edge.__table__.c.x1.nullable)
+ is_true(Edge.__table__.c.y1.nullable)
+
+ decl_base.metadata.create_all(testing.db)
+
+ with Session(testing.db) as sess:
+ sess.add(Edge(start=None))
+ sess.commit()
+
+ if disable_none_on:
+ eq_(
+ sess.execute(select(Edge.start)).scalar_one(),
+ Point(x=None, y=None),
+ )
+ else:
+ eq_(
+ sess.execute(select(Edge.start)).scalar_one(),
+ None,
+ )
+
class AllYourFavoriteHitsTest(fixtures.TestBase, testing.AssertsCompiledSQL):
"""try a bunch of common mappings using the new style"""
import dataclasses
import operator
import random
+from typing import Optional
import sqlalchemy as sa
from sqlalchemy import asc
from sqlalchemy.orm import configure_mappers
from sqlalchemy.orm import defer
from sqlalchemy.orm import load_only
+from sqlalchemy.orm import Mapped
from sqlalchemy.orm import mapped_column
from sqlalchemy.orm import relationship
from sqlalchemy.orm import Session
def test_not_none(self):
Edge = self.classes.Edge
+ Point = self.classes.Point
- # current contract. the composite is None
- # when hasn't been populated etc. on a
- # pending/transient object.
+ # new in 2.1; None return can be controlled, so by default you
+ # get an empty populated object
e1 = Edge()
- assert e1.end is None
+ eq_(e1.end, Point(None, None))
sess = fixture_session()
sess.add(e1)
- # however, once it's persistent, the code as of 0.7.3
- # would unconditionally populate it, even though it's
- # all None. I think this usage contract is inconsistent,
- # and it would be better that the composite is just
- # created unconditionally in all cases.
- # but as we are just trying to fix [ticket:2308] and
- # [ticket:2309] without changing behavior we maintain
- # that only "persistent" gets the composite with the
- # Nones
+ # old notes here referred to 0.7.3 as well as issue #2308, #2309.
+ # however as of 2.1 this is consistent
sess.flush()
- assert e1.end is not None
+ eq_(e1.end, Point(None, None))
def test_eager_load(self):
Graph, Point = self.classes.Graph, self.classes.Point
def test_default_value(self):
Edge = self.classes.Edge
+ Point = self.classes.Point
e = Edge()
- eq_(e.start, None)
+ eq_(e.start, Point(None, None))
def test_no_name_declarative(self, decl_base, connection):
"""test #7751"""
(
LoaderCallableStatus.NO_VALUE
if not active_history
- else None
+ else Point(None, None)
),
Edge.start.impl,
)
"SELECT edge.id, edge.x1, edge.y1, edge.x2, edge.y2 FROM edge "
"ORDER BY edge.x1, edge.y1",
)
+
+
+class NoneReturnTest(fixtures.TestBase):
+
+ @testing.fixture
+ def edge_point_fixture(self, decl_base):
+ @dataclasses.dataclass
+ class Point:
+ x: Optional[int]
+ y: Optional[int]
+
+ def go(return_none_on):
+ class Edge(decl_base):
+ __tablename__ = "edge"
+ id: Mapped[int] = mapped_column(primary_key=True)
+ start = composite(Point, return_none_on=return_none_on)
+
+ return Point, Edge
+
+ return go
+
+ @testing.fixture
+ def edge_point_persist_fixture(self, edge_point_fixture, decl_base):
+ def go(return_none_on):
+ Point, Edge = edge_point_fixture(return_none_on)
+
+ decl_base.metadata.create_all(testing.db)
+
+ with Session(testing.db) as sess:
+ sess.add(Edge(x=None, y=None))
+ sess.commit()
+ return Point, Edge
+
+ return go
+
+ def test_special_rule(self, edge_point_fixture):
+ Point, Edge = edge_point_fixture(lambda x, y: y is None)
+
+ obj = Edge()
+ eq_(obj.start, None)
+
+ obj = Edge(y=5)
+ eq_(obj.start, Point(x=None, y=5))
+
+ obj = Edge(y=5, x=7)
+ eq_(obj.start, Point(x=7, y=5))
+
+ obj = Edge(y=None, x=7)
+ eq_(obj.start, None)
+
+ @testing.variation("return_none_on", [True, False])
+ def test_pending_object_no_return_none(
+ self, edge_point_fixture, return_none_on
+ ):
+ Point, Edge = edge_point_fixture(
+ (lambda *args: all(arg is None for arg in args))
+ if return_none_on
+ else None
+ )
+
+ obj = Edge()
+
+ if return_none_on:
+ eq_(obj.start, None)
+ else:
+ eq_(obj.start, Point(x=None, y=None))
+
+ # object stays in place since it was assigned. this is to support
+ # in-place mutation of the object
+ obj.x = 5
+ if return_none_on:
+ eq_(obj.start, None)
+ else:
+ eq_(obj.start, Point(x=None, y=None))
+
+ # only if we pop from the dict can we change that
+ obj.__dict__.pop("start")
+ eq_(obj.start, Point(x=5, y=None))
+
+ obj.x = None
+ obj.__dict__.pop("start")
+ if return_none_on:
+ eq_(obj.start, None)
+ else:
+ eq_(obj.start, Point(x=None, y=None))
+
+ @testing.variation("return_none_on", [True, False])
+ def test_query_from_composite_directly(
+ self, edge_point_persist_fixture, return_none_on
+ ):
+ Point, Edge = edge_point_persist_fixture(
+ (lambda *args: all(arg is None for arg in args))
+ if return_none_on
+ else None
+ )
+
+ with Session(testing.db) as sess:
+ value = sess.scalar(select(Edge.start))
+
+ if return_none_on:
+ eq_(value, None)
+ else:
+ eq_(value, Point(x=None, y=None))
+
+ @testing.variation("return_none_on", [True, False])
+ def test_access_on_persistent(
+ self, edge_point_persist_fixture, return_none_on
+ ):
+ Point, Edge = edge_point_persist_fixture(
+ (lambda *args: all(arg is None for arg in args))
+ if return_none_on
+ else None
+ )
+
+ with Session(testing.db) as sess:
+ edge = sess.scalars(select(Edge)).one()
+
+ if return_none_on:
+ eq_(edge.start, None)
+ else:
+ eq_(edge.start, Point(x=None, y=None))
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
