Merge "warn or deprecate for auto-aliasing in joins"

7 files changed, 161 insertions, 51 deletions

diff --git a/lib/sqlalchemy/exc.py b/lib/sqlalchemy/exc.py
index a24cf7ba4..bcec7d30d 100644
--- a/lib/sqlalchemy/exc.py
+++ b/lib/sqlalchemy/exc.py
@@ -43,6 +43,12 @@ class HasDescriptionCode(object):
                 )
             )
 
+    def __str__(self):
+        message = super(HasDescriptionCode, self).__str__()
+        if self.code:
+            message = "%s %s" % (message, self._code_str())
+        return message
+
 
 class SQLAlchemyError(HasDescriptionCode, Exception):
     """Generic error class."""
@@ -660,12 +666,6 @@ class SADeprecationWarning(HasDescriptionCode, DeprecationWarning):
     deprecated_since = None
     "Indicates the version that started raising this deprecation warning"
 
-    def __str__(self):
-        message = super(SADeprecationWarning, self).__str__()
-        if self.code:
-            message = "%s %s" % (message, self._code_str())
-        return message
-
 
 class RemovedIn20Warning(SADeprecationWarning):
     """Issued for usage of APIs specifically deprecated in SQLAlchemy 2.0.
diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index 85c736e12..45df57c5d 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -1951,6 +1951,18 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
 
                 # make the right hand side target into an ORM entity
                 right = aliased(right_mapper, right_selectable)
+
+                util.warn_deprecated(
+                    "An alias is being generated automatically against "
+                    "joined entity %s for raw clauseelement, which is "
+                    "deprecated and will be removed in a later release. "
+                    "Use the aliased() "
+                    "construct explicitly, see the linked example."
+                    % right_mapper,
+                    "1.4",
+                    code="xaj1",
+                )
+
             elif create_aliases:
                 # it *could* work, but it doesn't right now and I'd rather
                 # get rid of aliased=True completely
@@ -1975,6 +1987,18 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
             right = aliased(right, flat=True)
             need_adapter = True
 
+            if not create_aliases:
+                util.warn(
+                    "An alias is being generated automatically against "
+                    "joined entity %s due to overlapping tables.  This is a "
+                    "legacy pattern which may be "
+                    "deprecated in a later release.  Use the "
+                    "aliased(<entity>, flat=True) "
+                    "construct explicitly, see the linked example."
+                    % right_mapper,
+                    code="xaj2",
+                )
+
         if need_adapter:
             assert right_mapper
 
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 5eee134d5..7f111b237 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1011,9 +1011,52 @@ class Mapper(
                         # immediate table of the inherited mapper, not its
                         # full table which could pull in other stuff we don't
                         # want (allows test/inheritance.InheritTest4 to pass)
-                        self.inherit_condition = sql_util.join_condition(
-                            self.inherits.local_table, self.local_table
-                        )
+                        try:
+                            self.inherit_condition = sql_util.join_condition(
+                                self.inherits.local_table, self.local_table
+                            )
+                        except sa_exc.NoForeignKeysError as nfe:
+                            assert self.inherits.local_table is not None
+                            assert self.local_table is not None
+                            util.raise_(
+                                sa_exc.NoForeignKeysError(
+                                    "Can't determine the inherit condition "
+                                    "between inherited table '%s' and "
+                                    "inheriting "
+                                    "table '%s'; tables have no "
+                                    "foreign key relationships established.  "
+                                    "Please ensure the inheriting table has "
+                                    "a foreign key relationship to the "
+                                    "inherited "
+                                    "table, or provide an "
+                                    "'on clause' using "
+                                    "the 'inherit_condition' mapper argument."
+                                    % (
+                                        self.inherits.local_table.description,
+                                        self.local_table.description,
+                                    )
+                                ),
+                                replace_context=nfe,
+                            )
+                        except sa_exc.AmbiguousForeignKeysError as afe:
+                            assert self.inherits.local_table is not None
+                            assert self.local_table is not None
+                            util.raise_(
+                                sa_exc.AmbiguousForeignKeysError(
+                                    "Can't determine the inherit condition "
+                                    "between inherited table '%s' and "
+                                    "inheriting "
+                                    "table '%s'; tables have more than one "
+                                    "foreign key relationship established.  "
+                                    "Please specify the 'on clause' using "
+                                    "the 'inherit_condition' mapper argument."
+                                    % (
+                                        self.inherits.local_table.description,
+                                        self.local_table.description,
+                                    )
+                                ),
+                                replace_context=afe,
+                            )
                     self.persist_selectable = sql.join(
                         self.inherits.persist_selectable,
                         self.local_table,
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 01a8becc3..63a2774b3 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -1172,6 +1172,14 @@ def aliased(element, alias=None, name=None, flat=False, adapt_on_names=False):
     :class:`_expression.Alias` is not
     ORM-mapped in this case.
 
+    .. seealso::
+
+        :ref:`tutorial_orm_entity_aliases` - in the :ref:`unified_tutorial`
+
+        :ref:`orm_queryguide_orm_aliases` - in the :ref:`queryguide_toplevel`
+
+    :ref:`ormtutorial_aliases` - in the legacy :ref:`ormtutorial_toplevel`
+
     :param element: element to be aliased.  Is normally a mapped class,
      but for convenience can also be a :class:`_expression.FromClause`
      element.
diff --git a/lib/sqlalchemy/sql/roles.py b/lib/sqlalchemy/sql/roles.py
index b9010397c..70ad4cefa 100644
--- a/lib/sqlalchemy/sql/roles.py
+++ b/lib/sqlalchemy/sql/roles.py
@@ -145,7 +145,10 @@ class FromClauseRole(ColumnsClauseRole, JoinTargetRole):
 
 class StrictFromClauseRole(FromClauseRole):
     # does not allow text() or select() objects
-    pass
+
+    @property
+    def description(self):
+        raise NotImplementedError()
 
 
 class AnonymizedFromClauseRole(StrictFromClauseRole):
diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py
index 0cf0cbc7a..986dbb5e9 100644
--- a/lib/sqlalchemy/testing/assertions.py
+++ b/lib/sqlalchemy/testing/assertions.py
@@ -133,6 +133,11 @@ def uses_deprecated(*messages):
     return decorate
 
 
+_FILTERS = None
+_SEEN = None
+_EXC_CLS = None
+
+
 @contextlib.contextmanager
 def _expect_warnings(
     exc_cls,
@@ -143,58 +148,76 @@ def _expect_warnings(
     raise_on_any_unexpected=False,
 ):
 
+    global _FILTERS, _SEEN, _EXC_CLS
+
     if regex:
         filters = [re.compile(msg, re.I | re.S) for msg in messages]
     else:
-        filters = messages
-
-    seen = set(filters)
+        filters = list(messages)
+
+    if _FILTERS is not None:
+        # nested call; update _FILTERS and _SEEN, return.  outer
+        # block will assert our messages
+        assert _SEEN is not None
+        assert _EXC_CLS is not None
+        _FILTERS.extend(filters)
+        _SEEN.update(filters)
+        _EXC_CLS += (exc_cls,)
+        yield
+    else:
+        seen = _SEEN = set(filters)
+        _FILTERS = filters
+        _EXC_CLS = (exc_cls,)
 
-    if raise_on_any_unexpected:
+        if raise_on_any_unexpected:
 
-        def real_warn(msg, *arg, **kw):
-            raise AssertionError("Got unexpected warning: %r" % msg)
+            def real_warn(msg, *arg, **kw):
+                raise AssertionError("Got unexpected warning: %r" % msg)
 
-    else:
-        real_warn = warnings.warn
-
-    def our_warn(msg, *arg, **kw):
-        if isinstance(msg, exc_cls):
-            exception = type(msg)
-            msg = str(msg)
-        elif arg:
-            exception = arg[0]
         else:
-            exception = None
+            real_warn = warnings.warn
 
-        if not exception or not issubclass(exception, exc_cls):
-            return real_warn(msg, *arg, **kw)
+        def our_warn(msg, *arg, **kw):
 
-        if not filters and not raise_on_any_unexpected:
-            return
+            if isinstance(msg, _EXC_CLS):
+                exception = type(msg)
+                msg = str(msg)
+            elif arg:
+                exception = arg[0]
+            else:
+                exception = None
 
-        for filter_ in filters:
-            if (regex and filter_.match(msg)) or (
-                not regex and filter_ == msg
-            ):
-                seen.discard(filter_)
-                break
-        else:
-            real_warn(msg, *arg, **kw)
-
-    with mock.patch("warnings.warn", our_warn), mock.patch(
-        "sqlalchemy.util.SQLALCHEMY_WARN_20", True
-    ), mock.patch(
-        "sqlalchemy.util.deprecations.SQLALCHEMY_WARN_20", True
-    ), mock.patch(
-        "sqlalchemy.engine.row.LegacyRow._default_key_style", 2
-    ):
-        yield
+            if not exception or not issubclass(exception, _EXC_CLS):
+                return real_warn(msg, *arg, **kw)
 
-    if assert_ and (not py2konly or not compat.py3k):
-        assert not seen, "Warnings were not seen: %s" % ", ".join(
-            "%r" % (s.pattern if regex else s) for s in seen
-        )
+            if not filters and not raise_on_any_unexpected:
+                return
+
+            for filter_ in filters:
+                if (regex and filter_.match(msg)) or (
+                    not regex and filter_ == msg
+                ):
+                    seen.discard(filter_)
+                    break
+            else:
+                real_warn(msg, *arg, **kw)
+
+        with mock.patch("warnings.warn", our_warn), mock.patch(
+            "sqlalchemy.util.SQLALCHEMY_WARN_20", True
+        ), mock.patch(
+            "sqlalchemy.util.deprecations.SQLALCHEMY_WARN_20", True
+        ), mock.patch(
+            "sqlalchemy.engine.row.LegacyRow._default_key_style", 2
+        ):
+            try:
+                yield
+            finally:
+                _SEEN = _FILTERS = _EXC_CLS = None
+
+                if assert_ and (not py2konly or not compat.py3k):
+                    assert not seen, "Warnings were not seen: %s" % ", ".join(
+                        "%r" % (s.pattern if regex else s) for s in seen
+                    )
 
 
 def global_cleanup_assertions():
diff --git a/lib/sqlalchemy/testing/fixtures.py b/lib/sqlalchemy/testing/fixtures.py
index e6af0c546..01a838c56 100644
--- a/lib/sqlalchemy/testing/fixtures.py
+++ b/lib/sqlalchemy/testing/fixtures.py
@@ -526,6 +526,15 @@ class TablesTest(TestBase):
                 )
 
 
+class NoCache(object):
+    @config.fixture(autouse=True, scope="function")
+    def _disable_cache(self):
+        _cache = config.db._compiled_cache
+        config.db._compiled_cache = None
+        yield
+        config.db._compiled_cache = _cache
+
+
 class RemovesEvents(object):
     @util.memoized_property
     def _event_fns(self):