warn for all unmapped expressions

Expanded the warning emitted when a plain :func:`_sql.column` object is
present in a Declarative mapping to include any arbitrary SQL expression
that is not declared within an appropriate property type such as
:func:`_orm.column_property`, :func:`_orm.deferred`, etc. These attributes
are otherwise not mapped at all and remain unchanged within the class
dictionary. As it seems likely that such an expression is usually not
what's intended, this case now warns for all such otherwise ignored
expressions, rather than just the :func:`_sql.column` case.

Fixes: #9537
Change-Id: Ic4ca7a071a28adf4ea8680690025d927522a0805

diff --git a/doc/build/changelog/unreleased_20/9537.rst b/doc/build/changelog/unreleased_20/9537.rst
new file mode 100644 (file)
index 0000000..9d39d47
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9537.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 9537
+
+    Expanded the warning emitted when a plain :func:`_sql.column` object is
+    present in a Declarative mapping to include any arbitrary SQL expression
+    that is not declared within an appropriate property type such as
+    :func:`_orm.column_property`, :func:`_orm.deferred`, etc. These attributes
+    are otherwise not mapped at all and remain unchanged within the class
+    dictionary. As it seems likely that such an expression is usually not
+    what's intended, this case now warns for all such otherwise ignored
+    expressions, rather than just the :func:`_sql.column` case.

diff --git a/lib/sqlalchemy/orm/decl_base.py b/lib/sqlalchemy/orm/decl_base.py
index d01aad439533e6399733fdc1c44d0a9670637863..6be5142765b167169b9f80cbf48998c42b42a44f 100644 (file)
--- a/lib/sqlalchemy/orm/decl_base.py
+++ b/lib/sqlalchemy/orm/decl_base.py
@@ -1277,11 +1277,14 @@ class _ClassScanMapperConfig(_MapperConfig):
     def _warn_for_decl_attributes(
         self, cls: Type[Any], key: str, c: Any
     ) -> None:
-        if isinstance(c, expression.ColumnClause):
+        if isinstance(c, expression.ColumnElement):
             util.warn(
                 f"Attribute '{key}' on class {cls} appears to "
-                "be a non-schema 'sqlalchemy.sql.column()' "
-                "object; this won't be part of the declarative mapping"
+                "be a non-schema SQLAlchemy expression "
+                "object; this won't be part of the declarative mapping. "
+                "To map arbitrary expressions, use ``column_property()`` "
+                "or a similar function such as ``deferred()``, "
+                "``query_expression()`` etc. "
             )
 
     def _produce_column_copies(

diff --git a/test/orm/declarative/test_basic.py b/test/orm/declarative/test_basic.py
index 4aca4daa6d09ab5832c8c87ddf011d1d5df78c7e..2d712c823e9f0897b622fc0f5f7c54ec197104f8 100644 (file)
--- a/test/orm/declarative/test_basic.py
+++ b/test/orm/declarative/test_basic.py
@@ -1301,10 +1301,10 @@ class DeclarativeMultiBaseTest(
             class_mapper(Bar).get_property("some_data").columns[0] is t.c.data
         )
 
-    def test_lower_case_c_column_warning(self):
+    def test_non_sql_expression_warning_one(self):
         with assertions.expect_warnings(
             r"Attribute 'x' on class <class .*Foo.* appears to be a "
-            r"non-schema 'sqlalchemy.sql.column\(\)' object; "
+            r"non-schema SQLAlchemy expression object; "
         ):
 
             class Foo(Base):
@@ -1314,13 +1314,14 @@ class DeclarativeMultiBaseTest(
                 x = sa.sql.expression.column(Integer)
                 y = Column(Integer)
 
+    def test_non_sql_expression_warning_two(self):
         class MyMixin:
             x = sa.sql.expression.column(Integer)
             y = Column(Integer)
 
         with assertions.expect_warnings(
             r"Attribute 'x' on class <class .*MyMixin.* appears to be a "
-            r"non-schema 'sqlalchemy.sql.column\(\)' object; "
+            r"non-schema SQLAlchemy expression object; "
         ):
 
             class Foo2(MyMixin, Base):
@@ -1328,9 +1329,10 @@ class DeclarativeMultiBaseTest(
 
                 id = Column(Integer, primary_key=True)
 
+    def test_non_sql_expression_warning_three(self):
         with assertions.expect_warnings(
             r"Attribute 'x' on class <class .*Foo3.* appears to be a "
-            r"non-schema 'sqlalchemy.sql.column\(\)' object; "
+            r"non-schema SQLAlchemy expression object; "
         ):
 
             class Foo3(Base):
@@ -1344,9 +1346,10 @@ class DeclarativeMultiBaseTest(
 
                 y = Column(Integer)
 
+    def test_non_sql_expression_warning_four(self):
         with assertions.expect_warnings(
             r"Attribute 'x' on class <class .*Foo4.* appears to be a "
-            r"non-schema 'sqlalchemy.sql.column\(\)' object; "
+            r"non-schema SQLAlchemy expression object; "
         ):
 
             class MyMixin2:
@@ -1361,6 +1364,25 @@ class DeclarativeMultiBaseTest(
 
                 id = Column(Integer, primary_key=True)
 
+    def test_non_sql_expression_warning_five(self):
+
+        # test for #9537
+        with assertions.expect_warnings(
+            r"Attribute 'x' on class <class .*Foo5.* appears to be a "
+            r"non-schema SQLAlchemy expression object; ",
+            r"Attribute 'y' on class <class .*Foo5.* appears to be a "
+            r"non-schema SQLAlchemy expression object; ",
+            raise_on_any_unexpected=True,
+        ):
+
+            class Foo5(Base):
+                __tablename__ = "foo5"
+
+                id = Column(Integer, primary_key=True)
+                x = Column("x", String()).collate("some collation")
+                y = Column("y", Integer) + 5
+                z = "im not a sqlalchemy thing"
+
     def test_column_named_twice(self):
         with expect_warnings(
             "On class 'Foo', Column object 'x' named directly multiple "

diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py
index 99cfdc3837935c2bb8180990c6fd0e24b4cb0530..27f1d705258def9659c097ac2bebf58dbd42bb36 100644 (file)
--- a/test/orm/inheritance/test_basic.py
+++ b/test/orm/inheritance/test_basic.py
@@ -198,11 +198,9 @@ class PolyExpressionEagerLoad(fixtures.DeclarativeMappedTest):
             child_id = Column(Integer, ForeignKey("a.id"))
             child = relationship("A")
 
-            p_a = case((discriminator == "a", "a"), else_="b")
-
             __mapper_args__ = {
                 "polymorphic_identity": "a",
-                "polymorphic_on": p_a,
+                "polymorphic_on": case((discriminator == "a", "a"), else_="b"),
             }
 
         class B(A):