From: Federico Caselli Date: Mon, 12 Jun 2023 20:55:15 +0000 (+0200) Subject: Fixed regression with callables as daclasses defaults X-Git-Tag: rel_2_0_17~15^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=017a43cb01b5bf93cd262e930627fb07410ef387;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Fixed regression with callables as daclasses defaults Fixed regression introduced in 2.0.16 by :ticket:`9879` where passing a callable to the :paramref:`_orm.mapped_column.default` parameter of :class:`_orm.mapped_column` while also setting ``init=False`` would interpret this value as a Dataclass default value which would be assigned directly to new instances of the object directly, bypassing the default generator taking place as the as the :paramref:`_schema.Column.default` value generator on the underlying :class:`_schema.Column`. This condition is now detected so that the previous behavior is maintained, however a deprecation warning for this ambiguous use is emitted; to populate the default generator for a :class:`_schema.Column`, the :paramref:`_orm.mapped_column.insert_default` parameter should be used, which disambiguates from the :paramref:`_orm.mapped_column.default` parameter whose name is fixed as per pep-681. Fixes: #9936 Change-Id: I7e2c1c723e4711ec470336fcb4867f41c40f9d6b --- diff --git a/doc/build/changelog/unreleased_20/9936.rst b/doc/build/changelog/unreleased_20/9936.rst new file mode 100644 index 0000000000..6191d12a08 --- /dev/null +++ b/doc/build/changelog/unreleased_20/9936.rst @@ -0,0 +1,18 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 9936 + + Fixed regression introduced in 2.0.16 by :ticket:`9879` where passing a + callable to the :paramref:`_orm.mapped_column.default` parameter of + :class:`_orm.mapped_column` while also setting ``init=False`` would + interpret this value as a Dataclass default value which would be assigned + directly to new instances of the object directly, bypassing the default + generator taking place as the :paramref:`_schema.Column.default` + value generator on the underlying :class:`_schema.Column`. This condition + is now detected so that the previous behavior is maintained, however a + deprecation warning for this ambiguous use is emitted; to populate the + default generator for a :class:`_schema.Column`, the + :paramref:`_orm.mapped_column.insert_default` parameter should be used, + which disambiguates from the :paramref:`_orm.mapped_column.default` + parameter whose name is fixed as per pep-681. + diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index b99364031f..88cafcb645 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -69,6 +69,7 @@ from ..sql.cache_key import HasCacheKey from ..sql.operators import ColumnOperators from ..sql.schema import Column from ..sql.type_api import TypeEngine +from ..util import warn_deprecated from ..util.typing import RODescriptorReference from ..util.typing import TypedDict @@ -200,7 +201,7 @@ class _AttributeOptions(NamedTuple): dataclasses_compare: Union[_NoArg, bool] dataclasses_kw_only: Union[_NoArg, bool] - def _as_dataclass_field(self) -> Any: + def _as_dataclass_field(self, key: str) -> Any: """Return a ``dataclasses.Field`` object given these arguments.""" kw: Dict[str, Any] = {} @@ -217,11 +218,29 @@ class _AttributeOptions(NamedTuple): if self.dataclasses_kw_only is not _NoArg.NO_ARG: kw["kw_only"] = self.dataclasses_kw_only + if "default" in kw and callable(kw["default"]): + # callable defaults are ambiguous. deprecate them in favour of + # insert_default or default_factory. #9936 + warn_deprecated( + f"Callable object passed to the ``default`` parameter for " + f"attribute {key!r} in a ORM-mapped Dataclasses context is " + "ambiguous, " + "and this use will raise an error in a future release. " + "If this callable is intended to produce Core level INSERT " + "default values for an underlying ``Column``, use " + "the ``mapped_column.insert_default`` parameter instead. " + "To establish this callable as providing a default value " + "for instances of the dataclass itself, use the " + "``default_factory`` dataclasses parameter.", + "2.0", + ) + if ( "init" in kw and not kw["init"] and "default" in kw - and "default_factory" not in kw # illegal but let field raise + and not callable(kw["default"]) # ignore callable defaults. #9936 + and "default_factory" not in kw # illegal but let dc.field raise ): # fix for #9879 default = kw.pop("default") @@ -246,7 +265,7 @@ class _AttributeOptions(NamedTuple): """ if isinstance(elem, _DCAttributeOptions): - dc_field = elem._attribute_options._as_dataclass_field() + dc_field = elem._attribute_options._as_dataclass_field(key) return (key, annotation, dc_field) elif elem is not _NoArg.NO_ARG: diff --git a/test/orm/declarative/test_dc_transforms.py b/test/orm/declarative/test_dc_transforms.py index 96fedf1275..cbe08f30e1 100644 --- a/test/orm/declarative/test_dc_transforms.py +++ b/test/orm/declarative/test_dc_transforms.py @@ -49,6 +49,7 @@ from sqlalchemy.sql.base import _NoArg from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ from sqlalchemy.testing import eq_regex +from sqlalchemy.testing import expect_deprecated from sqlalchemy.testing import expect_raises from sqlalchemy.testing import expect_raises_message from sqlalchemy.testing import fixtures @@ -788,6 +789,38 @@ class DCTransformsTest(AssertsCompiledSQL, fixtures.TestBase): ne_(fields["def_no_init"].default_factory, dataclasses.MISSING) eq_(fields["call_no_init"].default_factory, c20) + def test_dataclass_default_callable(self, dc_decl_base): + """test for #9936""" + + def cd(): + return 42 + + with expect_deprecated( + "Callable object passed to the ``default`` parameter for " + "attribute 'value' in a ORM-mapped Dataclasses context is " + "ambiguous, and this use will raise an error in a future " + "release. If this callable is intended to produce Core level ", + "Callable object passed to the ``default`` parameter for " + "attribute 'no_init' in a ORM-mapped Dataclasses context is " + "ambiguous, and this use will raise an error in a future " + "release. If this callable is intended to produce Core level ", + ): + + class A(dc_decl_base): + __tablename__ = "a" + id: Mapped[int] = mapped_column(primary_key=True) + value: Mapped[int] = mapped_column(default=cd) + no_init: Mapped[int] = mapped_column(default=cd, init=False) + + a = A(id=100) + is_false("no_init" in a.__dict__) + eq_(a.value, cd) + eq_(a.no_init, None) + + fields = {f.name: f for f in dataclasses.fields(A)} + eq_(fields["value"].default, cd) + eq_(fields["no_init"].default, cd) + class RelationshipDefaultFactoryTest(fixtures.TestBase): def test_list(self, dc_decl_base: Type[MappedAsDataclass]):