From: Mike Bayer Date: Mon, 17 Mar 2025 20:46:12 +0000 (-0400) Subject: ensure SQL expressions w/o bool pass through to correct typing error X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b19a09812c2b0806cc063e42993216fc1ead6ed2;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git ensure SQL expressions w/o bool pass through to correct typing error Fixed regression which occurred as of 2.0.37 where the checked :class:`.ArgumentError` that's raised when an inappropriate type or object is used inside of a :class:`.Mapped` annotation would raise ``TypeError`` with "boolean value of this clause is not defined" if the object resolved into a SQL expression in a boolean context, for programs where future annotations mode was not enabled. This case is now handled explicitly and a new error message has also been tailored for this case. In addition, as there are at least half a dozen distinct error scenarios for intepretation of the :class:`.Mapped` construct, these scenarios have all been unified under a new subclass of :class:`.ArgumentError` called :class:`.MappedAnnotationError`, to provide some continuity between these different scenarios, even though specific messaging remains distinct. Fixes: #12329 Change-Id: I0193e3479c84a48b364df8655f050e2e84151122 --- diff --git a/doc/build/changelog/unreleased_20/12329.rst b/doc/build/changelog/unreleased_20/12329.rst new file mode 100644 index 0000000000..9e4d1519a5 --- /dev/null +++ b/doc/build/changelog/unreleased_20/12329.rst @@ -0,0 +1,16 @@ +.. change:: + :tags: bug, orm + :tickets: 12329 + + Fixed regression which occurred as of 2.0.37 where the checked + :class:`.ArgumentError` that's raised when an inappropriate type or object + is used inside of a :class:`.Mapped` annotation would raise ``TypeError`` + with "boolean value of this clause is not defined" if the object resolved + into a SQL expression in a boolean context, for programs where future + annotations mode was not enabled. This case is now handled explicitly and + a new error message has also been tailored for this case. In addition, as + there are at least half a dozen distinct error scenarios for intepretation + of the :class:`.Mapped` construct, these scenarios have all been unified + under a new subclass of :class:`.ArgumentError` called + :class:`.MappedAnnotationError`, to provide some continuity between these + different scenarios, even though specific messaging remains distinct. diff --git a/lib/sqlalchemy/orm/decl_base.py b/lib/sqlalchemy/orm/decl_base.py index a2291d2d75..9a1e752c43 100644 --- a/lib/sqlalchemy/orm/decl_base.py +++ b/lib/sqlalchemy/orm/decl_base.py @@ -1577,7 +1577,7 @@ class _ClassScanMapperConfig(_MapperConfig): is_dataclass, ) except NameError as ne: - raise exc.ArgumentError( + raise orm_exc.MappedAnnotationError( f"Could not resolve all types within mapped " f'annotation: "{annotation}". Ensure all ' f"types are written correctly and are " diff --git a/lib/sqlalchemy/orm/exc.py b/lib/sqlalchemy/orm/exc.py index 0494edf983..a2f7c9f78a 100644 --- a/lib/sqlalchemy/orm/exc.py +++ b/lib/sqlalchemy/orm/exc.py @@ -65,6 +65,15 @@ class FlushError(sa_exc.SQLAlchemyError): """A invalid condition was detected during flush().""" +class MappedAnnotationError(sa_exc.ArgumentError): + """Raised when ORM annotated declarative cannot interpret the + expression present inside of the :class:`.Mapped` construct. + + .. versionadded:: 2.0.40 + + """ + + class UnmappedError(sa_exc.InvalidRequestError): """Base for exceptions that involve expected mappings not present.""" diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index f120f0d03a..2923ca6e4f 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -28,6 +28,7 @@ from typing import TypeVar from typing import Union from . import attributes +from . import exc as orm_exc from . import strategy_options from .base import _DeclarativeMapped from .base import class_mapper @@ -56,6 +57,7 @@ from ..sql.type_api import TypeEngine from ..util.typing import de_optionalize_union_types from ..util.typing import get_args from ..util.typing import includes_none +from ..util.typing import is_a_type from ..util.typing import is_fwd_ref from ..util.typing import is_pep593 from ..util.typing import is_pep695 @@ -858,16 +860,23 @@ class MappedColumn( isinstance(our_type, type) and issubclass(our_type, TypeEngine) ): - raise sa_exc.ArgumentError( + raise orm_exc.MappedAnnotationError( f"The type provided inside the {self.column.key!r} " "attribute Mapped annotation is the SQLAlchemy type " f"{our_type}. Expected a Python type instead" ) - else: - raise sa_exc.ArgumentError( + elif is_a_type(our_type): + raise orm_exc.MappedAnnotationError( "Could not locate SQLAlchemy Core type for Python " f"type {our_type} inside the {self.column.key!r} " "attribute Mapped annotation" ) + else: + raise orm_exc.MappedAnnotationError( + f"The object provided inside the {self.column.key!r} " + "attribute Mapped annotation is not a Python type, " + f"it's the object {our_type!r}. Expected a Python " + "type." + ) self.column._set_type(new_sqltype) diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 4d4ce9b3e8..cf3d8772cc 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -36,6 +36,7 @@ import weakref from . import attributes # noqa from . import exc +from . import exc as orm_exc from ._typing import _O from ._typing import insp_is_aliased_class from ._typing import insp_is_mapper @@ -2299,7 +2300,7 @@ def _extract_mapped_subtype( if raw_annotation is None: if required: - raise sa_exc.ArgumentError( + raise orm_exc.MappedAnnotationError( f"Python typing annotation is required for attribute " f'"{cls.__name__}.{key}" when primary argument(s) for ' f'"{attr_cls.__name__}" construct are None or not present' @@ -2319,14 +2320,14 @@ def _extract_mapped_subtype( str_cleanup_fn=_cleanup_mapped_str_annotation, ) except _CleanupError as ce: - raise sa_exc.ArgumentError( + raise orm_exc.MappedAnnotationError( f"Could not interpret annotation {raw_annotation}. " "Check that it uses names that are correctly imported at the " "module level. See chained stack trace for more hints." ) from ce except NameError as ne: if raiseerr and "Mapped[" in raw_annotation: # type: ignore - raise sa_exc.ArgumentError( + raise orm_exc.MappedAnnotationError( f"Could not interpret annotation {raw_annotation}. " "Check that it uses names that are correctly imported at the " "module level. See chained stack trace for more hints." @@ -2355,7 +2356,7 @@ def _extract_mapped_subtype( ): return None - raise sa_exc.ArgumentError( + raise orm_exc.MappedAnnotationError( f'Type annotation for "{cls.__name__}.{key}" ' "can't be correctly interpreted for " "Annotated Declarative Table form. ORM annotations " @@ -2376,7 +2377,7 @@ def _extract_mapped_subtype( return annotated, None if len(annotated.__args__) != 1: - raise sa_exc.ArgumentError( + raise orm_exc.MappedAnnotationError( "Expected sub-type for Mapped[] annotation" ) diff --git a/lib/sqlalchemy/util/typing.py b/lib/sqlalchemy/util/typing.py index 8980a85062..a1fb5920b9 100644 --- a/lib/sqlalchemy/util/typing.py +++ b/lib/sqlalchemy/util/typing.py @@ -546,7 +546,22 @@ def includes_none(type_: Any) -> bool: return any(includes_none(t) for t in pep695_values(type_)) if is_newtype(type_): return includes_none(type_.__supertype__) - return type_ in (NoneFwd, NoneType, None) + try: + return type_ in (NoneFwd, NoneType, None) + except TypeError: + # if type_ is Column, mapped_column(), etc. the use of "in" + # resolves to ``__eq__()`` which then gives us an expression object + # that can't resolve to boolean. just catch it all via exception + return False + + +def is_a_type(type_: Any) -> bool: + return ( + isinstance(type_, type) + or hasattr(type_, "__origin__") + or type_.__module__ in ("typing", "typing_extensions") + or type(type_).__mro__[0].__module__ in ("typing", "typing_extensions") + ) def is_union(type_: Any) -> TypeGuard[ArgsTypeProtocol]: diff --git a/test/orm/declarative/test_tm_future_annotations_sync.py b/test/orm/declarative/test_tm_future_annotations_sync.py index d435e9547b..d7d9414661 100644 --- a/test/orm/declarative/test_tm_future_annotations_sync.py +++ b/test/orm/declarative/test_tm_future_annotations_sync.py @@ -13,6 +13,7 @@ import datetime from decimal import Decimal import enum import inspect as _py_inspect +import re import typing from typing import Any from typing import cast @@ -67,6 +68,7 @@ from sqlalchemy.orm import DeclarativeBase from sqlalchemy.orm import declared_attr from sqlalchemy.orm import deferred from sqlalchemy.orm import DynamicMapped +from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import foreign from sqlalchemy.orm import Mapped from sqlalchemy.orm import mapped_column @@ -613,19 +615,179 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): id: Mapped[int] = mapped_column(primary_key=True) data: Mapped[MyClass] = mapped_column() - def test_construct_lhs_sqlalchemy_type(self, decl_base): - with expect_raises_message( - sa_exc.ArgumentError, - "The type provided inside the 'data' attribute Mapped " - "annotation is the SQLAlchemy type .*BigInteger.*. Expected " - "a Python type instead", - ): + @testing.variation( + "argtype", + [ + "type", + ("column", testing.requires.python310), + ("mapped_column", testing.requires.python310), + "column_class", + "ref_to_type", + ("ref_to_column", testing.requires.python310), + ], + ) + def test_construct_lhs_sqlalchemy_type(self, decl_base, argtype): + """test for #12329. - class User(decl_base): - __tablename__ = "users" + of note here are all the different messages we have for when the + wrong thing is put into Mapped[], and in fact in #12329 we added + another one. - id: Mapped[int] = mapped_column(primary_key=True) - data: Mapped[BigInteger] = mapped_column() + This is a lot of different messages, but at the same time they + occur at different places in the interpretation of types. If + we were to centralize all these messages, we'd still likely end up + doing distinct messages for each scenario, so instead we added + a new ArgumentError subclass MappedAnnotationError that provides + some commonality to all of these cases. + + + """ + expect_future_annotations = "annotations" in globals() + + if argtype.type: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # properties.py -> _init_column_for_annotation, type is + # a SQL type + "The type provided inside the 'data' attribute Mapped " + "annotation is the SQLAlchemy type .*BigInteger.*. Expected " + "a Python type instead", + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[BigInteger] = mapped_column() + + elif argtype.column: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # util.py -> _extract_mapped_subtype + ( + re.escape( + "Could not interpret annotation " + "Mapped[Column('q', BigInteger)]." + ) + if expect_future_annotations + # properties.py -> _init_column_for_annotation, object is + # not a SQL type or a python type, it's just some object + else re.escape( + "The object provided inside the 'data' attribute " + "Mapped annotation is not a Python type, it's the " + "object Column('q', BigInteger(), table=None). " + "Expected a Python type." + ) + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[Column("q", BigInteger)] = ( # noqa: F821 + mapped_column() + ) + + elif argtype.mapped_column: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # properties.py -> _init_column_for_annotation, object is + # not a SQL type or a python type, it's just some object + # interestingly, this raises at the same point for both + # future annotations mode and legacy annotations mode + r"The object provided inside the 'data' attribute " + "Mapped annotation is not a Python type, it's the object " + r"\. " + "Expected a Python type.", + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + big_integer: Mapped[int] = mapped_column() + data: Mapped[big_integer] = mapped_column() + + elif argtype.column_class: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # properties.py -> _init_column_for_annotation, type is not + # a SQL type + re.escape( + "Could not locate SQLAlchemy Core type for Python type " + " inside the " + "'data' attribute Mapped annotation" + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[Column] = mapped_column() + + elif argtype.ref_to_type: + mytype = BigInteger + with expect_raises_message( + orm_exc.MappedAnnotationError, + ( + # decl_base.py -> _exract_mappable_attributes + re.escape( + "Could not resolve all types within mapped " + 'annotation: "Mapped[mytype]"' + ) + if expect_future_annotations + # properties.py -> _init_column_for_annotation, type is + # a SQL type + else re.escape( + "The type provided inside the 'data' attribute Mapped " + "annotation is the SQLAlchemy type " + ". " + "Expected a Python type instead" + ) + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[mytype] = mapped_column() + + elif argtype.ref_to_column: + mycol = Column("q", BigInteger) + + with expect_raises_message( + orm_exc.MappedAnnotationError, + # decl_base.py -> _exract_mappable_attributes + ( + re.escape( + "Could not resolve all types within mapped " + 'annotation: "Mapped[mycol]"' + ) + if expect_future_annotations + else + # properties.py -> _init_column_for_annotation, object is + # not a SQL type or a python type, it's just some object + re.escape( + "The object provided inside the 'data' attribute " + "Mapped " + "annotation is not a Python type, it's the object " + "Column('q', BigInteger(), table=None). " + "Expected a Python type." + ) + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[mycol] = mapped_column() + + else: + argtype.fail() def test_construct_rhs_type_override_lhs(self, decl_base): class Element(decl_base): @@ -925,9 +1087,9 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): else: with expect_raises_message( - exc.ArgumentError, - "Could not locate SQLAlchemy Core type for Python type " - f"{tat} inside the 'data' attribute Mapped annotation", + orm_exc.MappedAnnotationError, + r"Could not locate SQLAlchemy Core type for Python type .*tat " + "inside the 'data' attribute Mapped annotation", ): declare() @@ -1381,7 +1543,7 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): text = ".*NewType.*" with expect_raises_message( - exc.ArgumentError, + orm_exc.MappedAnnotationError, "Could not locate SQLAlchemy Core type for Python type " f"{text} inside the 'data_one' attribute Mapped annotation", ): @@ -2352,7 +2514,8 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): ) with expect_raises_message( - sa_exc.ArgumentError, "Could not locate SQLAlchemy Core type" + orm_exc.MappedAnnotationError, + "Could not locate SQLAlchemy Core type", ): class MyClass(Base): diff --git a/test/orm/declarative/test_typed_mapping.py b/test/orm/declarative/test_typed_mapping.py index 6700cde56c..cb7712862d 100644 --- a/test/orm/declarative/test_typed_mapping.py +++ b/test/orm/declarative/test_typed_mapping.py @@ -4,6 +4,7 @@ import datetime from decimal import Decimal import enum import inspect as _py_inspect +import re import typing from typing import Any from typing import cast @@ -58,6 +59,7 @@ from sqlalchemy.orm import DeclarativeBase from sqlalchemy.orm import declared_attr from sqlalchemy.orm import deferred from sqlalchemy.orm import DynamicMapped +from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import foreign from sqlalchemy.orm import Mapped from sqlalchemy.orm import mapped_column @@ -604,19 +606,179 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): id: Mapped[int] = mapped_column(primary_key=True) data: Mapped[MyClass] = mapped_column() - def test_construct_lhs_sqlalchemy_type(self, decl_base): - with expect_raises_message( - sa_exc.ArgumentError, - "The type provided inside the 'data' attribute Mapped " - "annotation is the SQLAlchemy type .*BigInteger.*. Expected " - "a Python type instead", - ): + @testing.variation( + "argtype", + [ + "type", + ("column", testing.requires.python310), + ("mapped_column", testing.requires.python310), + "column_class", + "ref_to_type", + ("ref_to_column", testing.requires.python310), + ], + ) + def test_construct_lhs_sqlalchemy_type(self, decl_base, argtype): + """test for #12329. - class User(decl_base): - __tablename__ = "users" + of note here are all the different messages we have for when the + wrong thing is put into Mapped[], and in fact in #12329 we added + another one. - id: Mapped[int] = mapped_column(primary_key=True) - data: Mapped[BigInteger] = mapped_column() + This is a lot of different messages, but at the same time they + occur at different places in the interpretation of types. If + we were to centralize all these messages, we'd still likely end up + doing distinct messages for each scenario, so instead we added + a new ArgumentError subclass MappedAnnotationError that provides + some commonality to all of these cases. + + + """ + expect_future_annotations = "annotations" in globals() + + if argtype.type: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # properties.py -> _init_column_for_annotation, type is + # a SQL type + "The type provided inside the 'data' attribute Mapped " + "annotation is the SQLAlchemy type .*BigInteger.*. Expected " + "a Python type instead", + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[BigInteger] = mapped_column() + + elif argtype.column: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # util.py -> _extract_mapped_subtype + ( + re.escape( + "Could not interpret annotation " + "Mapped[Column('q', BigInteger)]." + ) + if expect_future_annotations + # properties.py -> _init_column_for_annotation, object is + # not a SQL type or a python type, it's just some object + else re.escape( + "The object provided inside the 'data' attribute " + "Mapped annotation is not a Python type, it's the " + "object Column('q', BigInteger(), table=None). " + "Expected a Python type." + ) + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[Column("q", BigInteger)] = ( # noqa: F821 + mapped_column() + ) + + elif argtype.mapped_column: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # properties.py -> _init_column_for_annotation, object is + # not a SQL type or a python type, it's just some object + # interestingly, this raises at the same point for both + # future annotations mode and legacy annotations mode + r"The object provided inside the 'data' attribute " + "Mapped annotation is not a Python type, it's the object " + r"\. " + "Expected a Python type.", + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + big_integer: Mapped[int] = mapped_column() + data: Mapped[big_integer] = mapped_column() + + elif argtype.column_class: + with expect_raises_message( + orm_exc.MappedAnnotationError, + # properties.py -> _init_column_for_annotation, type is not + # a SQL type + re.escape( + "Could not locate SQLAlchemy Core type for Python type " + " inside the " + "'data' attribute Mapped annotation" + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[Column] = mapped_column() + + elif argtype.ref_to_type: + mytype = BigInteger + with expect_raises_message( + orm_exc.MappedAnnotationError, + ( + # decl_base.py -> _exract_mappable_attributes + re.escape( + "Could not resolve all types within mapped " + 'annotation: "Mapped[mytype]"' + ) + if expect_future_annotations + # properties.py -> _init_column_for_annotation, type is + # a SQL type + else re.escape( + "The type provided inside the 'data' attribute Mapped " + "annotation is the SQLAlchemy type " + ". " + "Expected a Python type instead" + ) + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[mytype] = mapped_column() + + elif argtype.ref_to_column: + mycol = Column("q", BigInteger) + + with expect_raises_message( + orm_exc.MappedAnnotationError, + # decl_base.py -> _exract_mappable_attributes + ( + re.escape( + "Could not resolve all types within mapped " + 'annotation: "Mapped[mycol]"' + ) + if expect_future_annotations + else + # properties.py -> _init_column_for_annotation, object is + # not a SQL type or a python type, it's just some object + re.escape( + "The object provided inside the 'data' attribute " + "Mapped " + "annotation is not a Python type, it's the object " + "Column('q', BigInteger(), table=None). " + "Expected a Python type." + ) + ), + ): + + class User(decl_base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[mycol] = mapped_column() + + else: + argtype.fail() def test_construct_rhs_type_override_lhs(self, decl_base): class Element(decl_base): @@ -916,9 +1078,9 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): else: with expect_raises_message( - exc.ArgumentError, - "Could not locate SQLAlchemy Core type for Python type " - f"{tat} inside the 'data' attribute Mapped annotation", + orm_exc.MappedAnnotationError, + r"Could not locate SQLAlchemy Core type for Python type .*tat " + "inside the 'data' attribute Mapped annotation", ): declare() @@ -1372,7 +1534,7 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): text = ".*NewType.*" with expect_raises_message( - exc.ArgumentError, + orm_exc.MappedAnnotationError, "Could not locate SQLAlchemy Core type for Python type " f"{text} inside the 'data_one' attribute Mapped annotation", ): @@ -2343,7 +2505,8 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL): ) with expect_raises_message( - sa_exc.ArgumentError, "Could not locate SQLAlchemy Core type" + orm_exc.MappedAnnotationError, + "Could not locate SQLAlchemy Core type", ): class MyClass(Base):