Merge "warn for all unmapped expressions" into main

4 files changed, 46 insertions, 11 deletions

diff --git a/doc/build/changelog/unreleased_20/9537.rst b/doc/build/changelog/unreleased_20/9537.rst
new file mode 100644
index 000000000..9d39d479e
--- /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 d01aad439..6be514276 100644
--- 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 4aca4daa6..2d712c823 100644
--- 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 99cfdc383..27f1d7052 100644
--- 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):