warn or deprecate for auto-aliasing in joins

An extra layer of warning messages has been added to the functionality
of :meth:`_orm.Query.join` and the ORM version of
:meth:`_sql.Select.join`, where a few places where "automatic aliasing"
continues to occur will now be called out as a pattern to avoid, mostly
specific to the area of joined table inheritance where classes that share
common base tables are being joined together without using explicit aliases.
One case emits a legacy warning for a pattern that's not recommended,
the other case is fully deprecated.

The automatic aliasing within ORM join() which occurs for overlapping
mapped tables does not work consistently with all APIs such as
``contains_eager()``, and rather than continue to try to make these use
cases work everywhere, replacing with a more user-explicit pattern
is clearer, less prone to bugs and simplifies SQLAlchemy's internals
further.

The warnings include links to the errors.rst page where each pattern is
demonstrated along with the recommended pattern to fix.

* Improved the exception message generated when configuring a mapping with
  joined table inheritance where the two tables either have no foreign key
  relationships set up, or where they have multiple foreign key relationships
  set up. The message is now ORM specific and includes context that the
  :paramref:`_orm.Mapper.inherit_condition` parameter may be needed
  particularly for the ambiguous foreign keys case.

* Add explicit support in the _expect_warnings() assertion for nested
  _expect_warnings calls

* generalize the NoCache fixture, which we also need to catch warnings
  during compilation consistently

* generalize the __str__() method for the HasCode mixin so all warnings
  and errors include the code link in their string

Fixes: #6974
Change-Id: I84ed79ba2112c39eaab7973b6d6f46de7fa80842

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):