Merge "Raise at Core / ORM concrete inh level for label overlap"

3 files changed, 33 insertions, 11 deletions

diff --git a/lib/sqlalchemy/ext/declarative/extensions.py b/lib/sqlalchemy/ext/declarative/extensions.py
index 5c6356d8b..fd8bed6be 100644
--- a/lib/sqlalchemy/ext/declarative/extensions.py
+++ b/lib/sqlalchemy/ext/declarative/extensions.py
@@ -15,7 +15,6 @@ from ...orm import relationships
 from ...orm.base import _mapper_or_none
 from ...orm.clsregistry import _resolver
 from ...orm.decl_base import _DeferredMapperConfig
-from ...orm.decl_base import _get_immediate_cls_attr
 from ...orm.util import polymorphic_union
 from ...schema import Table
 from ...util import OrderedDict
@@ -86,6 +85,13 @@ class ConcreteBase(object):
        attribute to :class:`_declarative.ConcreteBase` so that the
        virtual discriminator column name can be customized.
 
+    .. versionchanged:: 1.4.2 The ``_concrete_discriminator_name`` attribute
+       need only be placed on the basemost class to take correct effect for
+       all subclasses.   An explicit error message is now raised if the
+       mapped column names conflict with the discriminator name, whereas
+       in the 1.3.x series there would be some warnings and then a non-useful
+       query would be generated.
+
     .. seealso::
 
         :class:`.AbstractConcreteBase`
@@ -112,8 +118,7 @@ class ConcreteBase(object):
             return
 
         discriminator_name = (
-            _get_immediate_cls_attr(cls, "_concrete_discriminator_name")
-            or "type"
+            getattr(cls, "_concrete_discriminator_name", None) or "type"
         )
 
         mappers = list(m.self_and_descendants)
@@ -264,8 +269,7 @@ class AbstractConcreteBase(ConcreteBase):
                 mappers.append(mn)
 
         discriminator_name = (
-            _get_immediate_cls_attr(cls, "_concrete_discriminator_name")
-            or "type"
+            getattr(cls, "_concrete_discriminator_name", None) or "type"
         )
         pjoin = cls._create_polymorphic_union(mappers, discriminator_name)
 
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 290f50236..37be077be 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -228,6 +228,15 @@ def polymorphic_union(
 
         m = {}
         for c in table.c:
+            if c.key == typecolname:
+                raise sa_exc.InvalidRequestError(
+                    "Polymorphic union can't use '%s' as the discriminator "
+                    "column due to mapped column %r; please apply the "
+                    "'typecolname' "
+                    "argument; this is available on "
+                    "ConcreteBase as '_concrete_discriminator_name'"
+                    % (typecolname, c)
+                )
             colnames.add(c.key)
             m[c.key] = c
             types[c.key] = c.type
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 29023c9fe..26c03b57b 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -4330,12 +4330,21 @@ class Label(roles.LabeledColumnExprRole, ColumnElement):
             disallow_is_literal=True,
             name_is_truncatable=isinstance(name, _truncated_label),
         )
-        # TODO: want to remove this assertion at some point.  all
-        # _make_proxy() implementations will give us back the key that
-        # is our "name" in the first place.  based on this we can
-        # safely return our "self.key" as the key here, to support a new
-        # case where the key and name are separate.
-        assert key == self.name
+
+        # there was a note here to remove this assertion, which was here
+        # to determine if we later could support a use case where
+        # the key and name of a label are separate.  But I don't know what
+        # that case was.  For now, this is an unexpected case that occurs
+        # when a label name conflicts with other columns and select()
+        # is attempting to disambiguate an explicit label, which is not what
+        # the user would want.   See issue #6090.
+        if key != self.name:
+            raise exc.InvalidRequestError(
+                "Label name %s is being renamed to an anonymous label due "
+                "to disambiguation "
+                "which is not supported right now.  Please use unique names "
+                "for explicit labels." % (self.name)
+            )
 
         e._propagate_attrs = selectable._propagate_attrs
         e._proxies.append(self)