From 6ce0d644db60ce6ea89eb15a76e078c4fa1a9066 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 4 Sep 2021 12:12:46 -0400 Subject: [PATCH] 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 --- doc/build/changelog/unreleased_14/6974.rst | 28 ++ .../changelog/unreleased_14/inh_cond.rst | 10 + doc/build/errors.rst | 176 ++++++++++ doc/build/orm/queryguide.rst | 2 + doc/build/orm/self_referential.rst | 3 +- lib/sqlalchemy/exc.py | 12 +- lib/sqlalchemy/orm/context.py | 24 ++ lib/sqlalchemy/orm/mapper.py | 49 ++- lib/sqlalchemy/orm/util.py | 8 + lib/sqlalchemy/sql/roles.py | 5 +- lib/sqlalchemy/testing/assertions.py | 105 +++--- lib/sqlalchemy/testing/fixtures.py | 9 + test/aaa_profiling/test_memusage.py | 2 +- test/aaa_profiling/test_orm.py | 15 +- test/orm/declarative/test_basic.py | 114 ------ test/orm/inheritance/test_assorted_poly.py | 20 +- test/orm/inheritance/test_basic.py | 43 ++- test/orm/inheritance/test_polymorphic_rel.py | 55 ++- test/orm/inheritance/test_relationship.py | 275 +++++++++++---- test/orm/inheritance/test_single.py | 82 +++-- test/orm/test_deprecations.py | 325 ++++++++++++++++-- test/orm/test_froms.py | 4 +- test/orm/test_joins.py | 63 ++-- test/orm/test_mapper.py | 60 +++- 24 files changed, 1081 insertions(+), 408 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/6974.rst create mode 100644 doc/build/changelog/unreleased_14/inh_cond.rst diff --git a/doc/build/changelog/unreleased_14/6974.rst b/doc/build/changelog/unreleased_14/6974.rst new file mode 100644 index 0000000000..fd6b970092 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6974.rst @@ -0,0 +1,28 @@ +.. change:: + :tags: bug, orm + :tickets: 6974, 6972 + + 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. + + .. seealso:: + + :ref:`error_xaj1` + + :ref:`error_xaj2` \ No newline at end of file diff --git a/doc/build/changelog/unreleased_14/inh_cond.rst b/doc/build/changelog/unreleased_14/inh_cond.rst new file mode 100644 index 0000000000..7a3f6d4109 --- /dev/null +++ b/doc/build/changelog/unreleased_14/inh_cond.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + + 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. + diff --git a/doc/build/errors.rst b/doc/build/errors.rst index a8e0939a49..da52623817 100644 --- a/doc/build/errors.rst +++ b/doc/build/errors.rst @@ -204,6 +204,181 @@ by passing ``True`` for the :paramref:`_orm.Session.future` parameter. :ref:`change_5150` - background on the change for SQLAlchemy 2.0. +.. _error_xaj1: + +An alias is being generated automatically for raw clauseelement +---------------------------------------------------------------- + +.. versionadded:: 1.4.26 + +This deprecation warning refers to a very old and likely not well known pattern +that applies to the legacy :meth:`_orm.Query.join` method as well as the +:term:`2.0 style` :meth:`_sql.Select.join` method, where a join can be stated +in terms of a :func:`_orm.relationship` but the target is the +:class:`_schema.Table` or other Core selectable to which the class is mapped, +rather than an ORM entity such as a mapped class or :func:`_orm.aliased` +construct:: + + a1 = Address.__table__ + + q = s.query(User).\ + join(a1, User.addresses).\ + filter(Address.email_address == 'ed@foo.com').all() + + +The above pattern also allows an arbitrary selectable, such as +a Core :class:`_sql.Join` or :class:`_sql.Alias` object, +however there is no automatic adaptation of this element, meaning the +Core element would need to be referred towards directly:: + + a1 = Address.__table__.alias() + + q = s.query(User).\ + join(a1, User.addresses).\ + filter(a1.c.email_address == 'ed@foo.com').all() + +The correct way to specify a join target is always by using the mapped +class itself or an :class:`_orm.aliased` object, in the latter case using the +:meth:`_orm.PropComparator.of_type` modifier to set up an alias:: + + # normal join to relationship entity + q = s.query(User).\ + join(User.addresses).\ + filter(Address.email_address == 'ed@foo.com') + + # name Address target explicitly, not necessary but legal + q = s.query(User).\ + join(Address, User.addresses).\ + filter(Address.email_address == 'ed@foo.com') + +Join to an alias:: + + from sqlalchemy.orm import aliased + + a1 = aliased(Address) + + # of_type() form; recommended + q = s.query(User).\ + join(User.addresses.of_type(a1)).\ + filter(a1.email_address == 'ed@foo.com') + + # target, onclause form + q = s.query(User).\ + join(a1, User.addresses).\ + filter(a1.email_address == 'ed@foo.com') + + +.. _error_xaj2: + +An alias is being generated automatically due to overlapping tables +------------------------------------------------------------------- + +.. versionadded:: 1.4.26 + +This warning is typically generated when querying using the +:meth:`_sql.Select.join` method or the legacy :meth:`_orm.Query.join` method +with mappings that involve joined table inheritance. The issue is that when +joining between two joined inheritance models that share a common base table, a +proper SQL JOIN between the two entities cannot be formed without applying an +alias to one side or the other; SQLAlchemy applies an alias to the right side +of the join. For example given a joined inheritance mapping as:: + + class Employee(Base): + __tablename__ = 'employee' + id = Column(Integer, primary_key=True) + manager_id = Column(ForeignKey("manager.id")) + name = Column(String(50)) + type = Column(String(50)) + + reports_to = relationship("Manager", foreign_keys=manager_id) + + __mapper_args__ = { + 'polymorphic_identity':'employee', + 'polymorphic_on':type, + } + + class Manager(Employee): + __tablename__ = 'manager' + id = Column(Integer, ForeignKey('employee.id'), primary_key=True) + + __mapper_args__ = { + 'polymorphic_identity':'manager', + 'inherit_condition': id == Employee.id + } + +The above mapping includes a relationship between the ``Employee`` and +``Manager`` classes. Since both classes make use of the "employee" database +table, from a SQL perspective this is a +:ref:`self referential relationship `. If we wanted to +query from both the ``Employee`` and ``Manager`` models using a join, at the +SQL level the "employee" table needs to be included twice in the query, which +means it must be aliased. When we create such a join using the SQLAlchemy +ORM, we get SQL that looks like the following: + +.. sourcecode:: pycon+sql + + >>> stmt = select(Employee, Manager).join(Employee.reports_to) + >>> print(stmt) + {opensql}SELECT employee.id, employee.manager_id, employee.name, + employee.type, manager_1.id AS id_1, employee_1.id AS id_2, + employee_1.manager_id AS manager_id_1, employee_1.name AS name_1, + employee_1.type AS type_1 + FROM employee JOIN + (employee AS employee_1 JOIN manager AS manager_1 ON manager_1.id = employee_1.id) + ON manager_1.id = employee.manager_id + +Above, the SQL selects FROM the ``employee`` table, representing the +``Employee`` entity in the query. It then joins to a right-nested join of +``employee AS employee_1 JOIN manager AS manager_1``, where the ``employee`` +table is stated again, except as an anonymous alias ``employee_1``. This is the +"automatic generation of an alias" that the warning message refers towards. + +When SQLAlchemy loads ORM rows that each contain an ``Employee`` and a +``Manager`` object, the ORM must adapt rows from what above is the +``employee_1`` and ``manager_1`` table aliases into those of the un-aliased +``Manager`` class. This process is internally complex and does not accommodate +for all API features, notably when trying to use eager loading features such as +:func:`_orm.contains_eager` with more deeply nested queries than are shown +here. As the pattern is unreliable for more complex scenarios and involves +implicit decisionmaking that is difficult to anticipate and follow, +the warning is emitted and this pattern may be considered a legacy feature. The +better way to write this query is to use the same patterns that apply to any +other self-referential relationship, which is to use the :func:`_orm.aliased` +construct explicitly. For joined-inheritance and other join-oriented mappings, +it is usually desirable to add the use of the :paramref:`_orm.aliased.flat` +parameter, which will allow a JOIN of two or more tables to be aliased by +applying an alias to the individual tables within the join, rather than +embedding the join into a new subquery: + +.. sourcecode:: pycon+sql + + >>> from sqlalchemy.orm import aliased + >>> manager_alias = aliased(Manager, flat=True) + >>> stmt = select(Employee, manager_alias).join(Employee.reports_to.of_type(manager_alias)) + >>> print(stmt) + {opensql}SELECT employee.id, employee.manager_id, employee.name, + employee.type, manager_1.id AS id_1, employee_1.id AS id_2, + employee_1.manager_id AS manager_id_1, employee_1.name AS name_1, + employee_1.type AS type_1 + FROM employee JOIN + (employee AS employee_1 JOIN manager AS manager_1 ON manager_1.id = employee_1.id) + ON manager_1.id = employee.manager_id + +If we then wanted to use :func:`_orm.contains_eager` to populate the +``reports_to`` attribute, we refer to the alias:: + + >>> stmt =select(Employee).join( + ... Employee.reports_to.of_type(manager_alias) + ... ).options( + ... contains_eager(Employee.reports_to.of_type(manager_alias)) + ... ) + +Without using the explicit :func:`_orm.aliased` object, in some more nested +cases the :func:`_orm.contains_eager` option does not have enough context to +know where to get its data from, in the case that the ORM is "auto-aliasing" +in a very nested context. Therefore it's best not to rely on this feature +and instead keep the SQL construction as explicit as possible. + Connections and Transactions ============================ @@ -907,6 +1082,7 @@ Mitigation of this error is via two general techniques: :ref:`session_expire` - background on attribute expiry + .. _error_7s2a: This Session's transaction has been rolled back due to a previous exception during flush diff --git a/doc/build/orm/queryguide.rst b/doc/build/orm/queryguide.rst index 04188232e0..2b1d9ae89b 100644 --- a/doc/build/orm/queryguide.rst +++ b/doc/build/orm/queryguide.rst @@ -261,6 +261,8 @@ views as well as custom column groupings such as mappings. :ref:`bundles` - in the ORM loading documentation. +.. _orm_queryguide_orm_aliases: + Selecting ORM Aliases ^^^^^^^^^^^^^^^^^^^^^ diff --git a/doc/build/orm/self_referential.rst b/doc/build/orm/self_referential.rst index e931c783cb..2f1c021020 100644 --- a/doc/build/orm/self_referential.rst +++ b/doc/build/orm/self_referential.rst @@ -4,7 +4,8 @@ Adjacency List Relationships ---------------------------- The **adjacency list** pattern is a common relational pattern whereby a table -contains a foreign key reference to itself. This is the most common +contains a foreign key reference to itself, in other words is a +**self referential relationship**. This is the most common way to represent hierarchical data in flat tables. Other methods include **nested sets**, sometimes called "modified preorder", as well as **materialized path**. Despite the appeal that modified preorder diff --git a/lib/sqlalchemy/exc.py b/lib/sqlalchemy/exc.py index a24cf7ba49..bcec7d30d6 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 85c736e12e..45df57c5d1 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(, 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 5eee134d53..7f111b2374 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 01a8becc31..63a2774b31 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 b9010397cf..70ad4cefa7 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 0cf0cbc7a7..986dbb5e9b 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 e6af0c546c..01a838c56d 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): diff --git a/test/aaa_profiling/test_memusage.py b/test/aaa_profiling/test_memusage.py index 6462ab9f1f..1c08c30a24 100644 --- a/test/aaa_profiling/test_memusage.py +++ b/test/aaa_profiling/test_memusage.py @@ -1141,7 +1141,7 @@ class MemUsageWBackendTest(EnsureZeroed): @profile_memory() def go(): - s = table2.select().subquery() + s = aliased(Bar, table2.select().subquery()) sess = session() sess.query(Foo).join(s, Foo.bars).all() sess.rollback() diff --git a/test/aaa_profiling/test_orm.py b/test/aaa_profiling/test_orm.py index 7de1811aed..727d48fcac 100644 --- a/test/aaa_profiling/test_orm.py +++ b/test/aaa_profiling/test_orm.py @@ -18,27 +18,14 @@ from sqlalchemy.orm import relationship from sqlalchemy.orm import selectinload from sqlalchemy.orm import Session from sqlalchemy.orm import sessionmaker -from sqlalchemy.testing import config from sqlalchemy.testing import fixtures from sqlalchemy.testing import profiling from sqlalchemy.testing.fixtures import fixture_session +from sqlalchemy.testing.fixtures import NoCache from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table -class NoCache(object): - run_setup_bind = "each" - - @classmethod - def setup_test_class(cls): - cls._cache = config.db._compiled_cache - config.db._compiled_cache = None - - @classmethod - def teardown_test_class(cls): - config.db._compiled_cache = cls._cache - - class MergeTest(NoCache, fixtures.MappedTest): __requires__ = ("python_profiling_backend",) diff --git a/test/orm/declarative/test_basic.py b/test/orm/declarative/test_basic.py index ecc824391d..4e2c465d84 100644 --- a/test/orm/declarative/test_basic.py +++ b/test/orm/declarative/test_basic.py @@ -2326,117 +2326,3 @@ class DeclarativeTest(DeclarativeTestBase): # Check to see if __init_subclass__ works in supported versions eq_(UserType._set_random_keyword_used_here, True) - - -# TODO: this should be using @combinations -def _produce_test(inline, stringbased): - class ExplicitJoinTest(fixtures.MappedTest): - @classmethod - def define_tables(cls, metadata): - global User, Address - Base = declarative_base(metadata=metadata) - - class User(Base, fixtures.ComparableEntity): - - __tablename__ = "users" - id = Column( - Integer, primary_key=True, test_needs_autoincrement=True - ) - name = Column(String(50)) - - class Address(Base, fixtures.ComparableEntity): - - __tablename__ = "addresses" - id = Column( - Integer, primary_key=True, test_needs_autoincrement=True - ) - email = Column(String(50)) - user_id = Column(Integer, ForeignKey("users.id")) - if inline: - if stringbased: - user = relationship( - "User", - primaryjoin="User.id==Address.user_id", - backref="addresses", - ) - else: - user = relationship( - User, - primaryjoin=User.id == user_id, - backref="addresses", - ) - - if not inline: - configure_mappers() - if stringbased: - Address.user = relationship( - "User", - primaryjoin="User.id==Address.user_id", - backref="addresses", - ) - else: - Address.user = relationship( - User, - primaryjoin=User.id == Address.user_id, - backref="addresses", - ) - - @classmethod - def insert_data(cls, connection): - params = [ - dict(list(zip(("id", "name"), column_values))) - for column_values in [ - (7, "jack"), - (8, "ed"), - (9, "fred"), - (10, "chuck"), - ] - ] - - connection.execute(User.__table__.insert(), params) - connection.execute( - Address.__table__.insert(), - [ - dict(list(zip(("id", "user_id", "email"), column_values))) - for column_values in [ - (1, 7, "jack@bean.com"), - (2, 8, "ed@wood.com"), - (3, 8, "ed@bettyboop.com"), - (4, 8, "ed@lala.com"), - (5, 9, "fred@fred.com"), - ] - ], - ) - - def test_aliased_join(self): - - # this query will screw up if the aliasing enabled in - # query.join() gets applied to the right half of the join - # condition inside the any(). the join condition inside of - # any() comes from the "primaryjoin" of the relationship, - # and should not be annotated with _orm_adapt. - # PropertyLoader.Comparator will annotate the left side with - # _orm_adapt, though. - - sess = fixture_session() - eq_( - sess.query(User) - .join(User.addresses, aliased=True) - .filter(Address.email == "ed@wood.com") - .filter(User.addresses.any(Address.email == "jack@bean.com")) - .all(), - [], - ) - - ExplicitJoinTest.__name__ = "ExplicitJoinTest%s%s" % ( - inline and "Inline" or "Separate", - stringbased and "String" or "Literal", - ) - return ExplicitJoinTest - - -for inline in True, False: - for stringbased in True, False: - testclass = _produce_test(inline, stringbased) - exec("%s = testclass" % testclass.__name__) - del testclass diff --git a/test/orm/inheritance/test_assorted_poly.py b/test/orm/inheritance/test_assorted_poly.py index cbf7b5042f..084305a7cc 100644 --- a/test/orm/inheritance/test_assorted_poly.py +++ b/test/orm/inheritance/test_assorted_poly.py @@ -2251,12 +2251,28 @@ class ColSubclassTest( id = Column(ForeignKey("a.id"), primary_key=True) x = MySpecialColumn(String) - def test_polymorphic_adaptation(self): + def test_polymorphic_adaptation_auto(self): A, B = self.classes.A, self.classes.B + s = fixture_session() + with testing.expect_warnings( + "An alias is being generated automatically " + "against joined entity mapped class B->b due to overlapping" + ): + self.assert_compile( + s.query(A).join(B).filter(B.x == "test"), + "SELECT a.id AS a_id FROM a JOIN " + "(a AS a_1 JOIN b AS b_1 ON a_1.id = b_1.id) " + "ON a.id = b_1.id WHERE b_1.x = :x_1", + ) + + def test_polymorphic_adaptation_manual_alias(self): + A, B = self.classes.A, self.classes.B + + b1 = aliased(B, flat=True) s = fixture_session() self.assert_compile( - s.query(A).join(B).filter(B.x == "test"), + s.query(A).join(b1).filter(b1.x == "test"), "SELECT a.id AS a_id FROM a JOIN " "(a AS a_1 JOIN b AS b_1 ON a_1.id = b_1.id) " "ON a.id = b_1.id WHERE b_1.x = :x_1", diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index 1b4a367ac1..1ca005bd09 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -3035,9 +3035,46 @@ class InhCondTest(fixtures.TestBase): mapper(Base, base_table) assert_raises_message( - sa_exc.ArgumentError, - "Can't find any foreign key relationships between " - "'base' and 'derived'.", + sa_exc.NoForeignKeysError, + "Can't determine the inherit condition between inherited table " + "'base' and inheriting table 'derived'; 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.", + mapper, + Derived, + derived_table, + inherits=Base, + ) + + def test_inh_cond_ambiguous_fk(self): + metadata = MetaData() + base_table = Table( + "base", + metadata, + Column("id", Integer, primary_key=True), + Column("favorite_derived", ForeignKey("derived.id")), + ) + derived_table = Table( + "derived", + metadata, + Column("id", Integer, ForeignKey("base.id"), primary_key=True), + ) + + class Base(object): + pass + + class Derived(Base): + pass + + mapper(Base, base_table) + assert_raises_message( + sa_exc.AmbiguousForeignKeysError, + "Can't determine the inherit condition between inherited table " + "'base' and inheriting table 'derived'; tables have more than " + "one foreign key relationship established. Please specify the " + "'on clause' using the 'inherit_condition' mapper argument.", mapper, Derived, derived_table, diff --git a/test/orm/inheritance/test_polymorphic_rel.py b/test/orm/inheritance/test_polymorphic_rel.py index de7723125a..1a96ee011e 100644 --- a/test/orm/inheritance/test_polymorphic_rel.py +++ b/test/orm/inheritance/test_polymorphic_rel.py @@ -13,6 +13,7 @@ from sqlalchemy.orm import subqueryload from sqlalchemy.orm import with_polymorphic from sqlalchemy.testing import assert_raises from sqlalchemy.testing import eq_ +from sqlalchemy.testing import fixtures from sqlalchemy.testing.assertsql import CompiledSQL from sqlalchemy.testing.fixtures import fixture_session from ._poly_fixtures import _Polymorphic @@ -29,8 +30,9 @@ from ._poly_fixtures import Paperwork from ._poly_fixtures import Person -class _PolymorphicTestBase(object): +class _PolymorphicTestBase(fixtures.NoCache): __backend__ = True + __dialect__ = "default_enhanced" @classmethod def setup_mappers(cls): @@ -1143,18 +1145,14 @@ class _PolymorphicTestBase(object): # non-polymorphic eq_(sess.query(Engineer).join(Person.paperwork).all(), [e1, e2, e3]) - def test_join_to_subclass(self): + def test_join_to_subclass_manual_alias(self): sess = fixture_session() - # TODO: these should all be deprecated (?) - these joins are on the - # core tables and should not be getting adapted, not sure why - # adaptation is happening? (is it?) emit a warning when the adaptation - # occurs? - + target = aliased(Engineer, people.join(engineers)) eq_( sess.query(Company) - .join(people.join(engineers), "employees") - .filter(Engineer.primary_language == "java") + .join(Company.employees.of_type(target)) + .filter(target.primary_language == "java") .all(), [c1], ) @@ -1169,16 +1167,6 @@ class _PolymorphicTestBase(object): [c1], ) - def test_join_to_subclass_two(self): - sess = fixture_session() - eq_( - sess.query(Company) - .join(people.join(engineers), "employees") - .filter(Engineer.primary_language == "java") - .all(), - [c1], - ) - def test_join_to_subclass_three(self): sess = fixture_session() ealias = aliased(Engineer) @@ -1192,9 +1180,10 @@ class _PolymorphicTestBase(object): def test_join_to_subclass_six(self): sess = fixture_session() + eq_( sess.query(Company) - .join(people.join(engineers), "employees") + .join(Company.employees.of_type(Engineer)) .join(Engineer.machines) .all(), [c1, c2], @@ -1202,23 +1191,25 @@ class _PolymorphicTestBase(object): def test_join_to_subclass_six_point_five(self): sess = fixture_session() - eq_( + + q = ( sess.query(Company) - .join(people.join(engineers), "employees") + .join(Company.employees.of_type(Engineer)) .join(Engineer.machines) .filter(Engineer.name == "dilbert") - .all(), - [c1], ) - - def test_join_to_subclass_seven(self): - sess = fixture_session() + self.assert_compile( + q, + "SELECT companies.company_id AS companies_company_id, " + "companies.name AS companies_name FROM companies JOIN " + "(people JOIN engineers ON people.person_id = " + "engineers.person_id) ON " + "companies.company_id = people.company_id " + "JOIN machines ON engineers.person_id = machines.engineer_id " + "WHERE people.name = :name_1", + ) eq_( - sess.query(Company) - .join(people.join(engineers), "employees") - .join(Engineer.machines) - .filter(Machine.name.ilike("%thinkpad%")) - .all(), + q.all(), [c1], ) diff --git a/test/orm/inheritance/test_relationship.py b/test/orm/inheritance/test_relationship.py index b9f3ac01d1..164b13f2e3 100644 --- a/test/orm/inheritance/test_relationship.py +++ b/test/orm/inheritance/test_relationship.py @@ -4,6 +4,7 @@ from sqlalchemy import Integer from sqlalchemy import select from sqlalchemy import String from sqlalchemy import testing +from sqlalchemy import util from sqlalchemy.orm import aliased from sqlalchemy.orm import backref from sqlalchemy.orm import configure_mappers @@ -55,6 +56,13 @@ class Paperwork(fixtures.ComparableEntity): pass +def _aliased_join_warning(arg): + return testing.expect_warnings( + "An alias is being generated automatically against joined entity " + "mapped class %s due to overlapping tables" % (arg,) + ) + + class SelfReferentialTestJoinedToBase(fixtures.MappedTest): run_setup_mappers = "once" @@ -275,7 +283,8 @@ class SelfReferentialJ2JTest(fixtures.MappedTest): Engineer(name="dilbert"), ) - def test_filter_aliasing(self): + @testing.combinations((True,), (False,), argnames="autoalias") + def test_filter_aliasing(self, autoalias): m1 = Manager(name="dogbert") m2 = Manager(name="foo") e1 = Engineer(name="wally", primary_language="java", reports_to=m1) @@ -287,32 +296,62 @@ class SelfReferentialJ2JTest(fixtures.MappedTest): sess.flush() sess.expunge_all() - # filter aliasing applied to Engineer doesn't whack Manager - eq_( - sess.query(Manager) - .join(Manager.engineers) - .filter(Manager.name == "dogbert") - .all(), - [m1], - ) + if autoalias: + # filter aliasing applied to Engineer doesn't whack Manager + with _aliased_join_warning("Engineer->engineers"): + eq_( + sess.query(Manager) + .join(Manager.engineers) + .filter(Manager.name == "dogbert") + .all(), + [m1], + ) - eq_( - sess.query(Manager) - .join(Manager.engineers) - .filter(Engineer.name == "dilbert") - .all(), - [m2], - ) + with _aliased_join_warning("Engineer->engineers"): + eq_( + sess.query(Manager) + .join(Manager.engineers) + .filter(Engineer.name == "dilbert") + .all(), + [m2], + ) - eq_( - sess.query(Manager, Engineer) - .join(Manager.engineers) - .order_by(Manager.name.desc()) - .all(), - [(m2, e2), (m1, e1)], - ) + with _aliased_join_warning("Engineer->engineers"): + eq_( + sess.query(Manager, Engineer) + .join(Manager.engineers) + .order_by(Manager.name.desc()) + .all(), + [(m2, e2), (m1, e1)], + ) + else: + eng = aliased(Engineer, flat=True) + eq_( + sess.query(Manager) + .join(Manager.engineers.of_type(eng)) + .filter(Manager.name == "dogbert") + .all(), + [m1], + ) - def test_relationship_compare(self): + eq_( + sess.query(Manager) + .join(Manager.engineers.of_type(eng)) + .filter(eng.name == "dilbert") + .all(), + [m2], + ) + + eq_( + sess.query(Manager, eng) + .join(Manager.engineers.of_type(eng)) + .order_by(Manager.name.desc()) + .all(), + [(m2, e2), (m1, e1)], + ) + + @testing.combinations((True,), (False,), argnames="autoalias") + def test_relationship_compare(self, autoalias): m1 = Manager(name="dogbert") m2 = Manager(name="foo") e1 = Engineer(name="dilbert", primary_language="java", reports_to=m1) @@ -328,21 +367,41 @@ class SelfReferentialJ2JTest(fixtures.MappedTest): sess.flush() sess.expunge_all() - eq_( - sess.query(Manager) - .join(Manager.engineers) - .filter(Engineer.reports_to == None) - .all(), # noqa - [], - ) + if autoalias: + with _aliased_join_warning("Engineer->engineers"): + eq_( + sess.query(Manager) + .join(Manager.engineers) + .filter(Engineer.reports_to == None) + .all(), + [], + ) - eq_( - sess.query(Manager) - .join(Manager.engineers) - .filter(Engineer.reports_to == m1) - .all(), - [m1], - ) + with _aliased_join_warning("Engineer->engineers"): + eq_( + sess.query(Manager) + .join(Manager.engineers) + .filter(Engineer.reports_to == m1) + .all(), + [m1], + ) + else: + eng = aliased(Engineer, flat=True) + eq_( + sess.query(Manager) + .join(Manager.engineers.of_type(eng)) + .filter(eng.reports_to == None) + .all(), + [], + ) + + eq_( + sess.query(Manager) + .join(Manager.engineers.of_type(eng)) + .filter(eng.reports_to == m1) + .all(), + [m1], + ) class SelfReferentialJ2JSelfTest(fixtures.MappedTest): @@ -728,16 +787,35 @@ class SelfReferentialM2MTest(fixtures.MappedTest, AssertsCompiledSQL): sess.add_all([c11, c12, c13, c21, c22, c23]) sess.flush() + # auto alias test: # test that the join to Child2 doesn't alias Child1 in the select stmt = select(Child1).join(Child1.left_child2) + + with _aliased_join_warning("Child2->child2"): + eq_( + set(sess.execute(stmt).scalars().unique()), + set([c11, c12, c13]), + ) + + with _aliased_join_warning("Child2->child2"): + eq_( + set(sess.query(Child1, Child2).join(Child1.left_child2)), + set([(c11, c22), (c12, c22), (c13, c23)]), + ) + + # manual alias test: + + c2 = aliased(Child2) + stmt = select(Child1).join(Child1.left_child2.of_type(c2)) + eq_( set(sess.execute(stmt).scalars().unique()), set([c11, c12, c13]), ) eq_( - set(sess.query(Child1, Child2).join(Child1.left_child2)), + set(sess.query(Child1, c2).join(Child1.left_child2.of_type(c2))), set([(c11, c22), (c12, c22), (c13, c23)]), ) @@ -748,16 +826,48 @@ class SelfReferentialM2MTest(fixtures.MappedTest, AssertsCompiledSQL): .join(Child2.right_children) .where(Child1.left_child2 == c22) ) + with _aliased_join_warning("Child1->child1"): + eq_( + set(sess.execute(stmt).scalars().unique()), + set([c22]), + ) + + # manual aliased version + c1 = aliased(Child1, flat=True) + stmt = ( + select(Child2) + .join(Child2.right_children.of_type(c1)) + .where(c1.left_child2 == c22) + ) eq_( set(sess.execute(stmt).scalars().unique()), set([c22]), ) # test the same again + with _aliased_join_warning("Child1->child1"): + self.assert_compile( + sess.query(Child2) + .join(Child2.right_children) + .filter(Child1.left_child2 == c22) + .statement, + "SELECT child2.id, parent.id AS id_1, parent.cls " + "FROM secondary AS secondary_1, parent " + "JOIN child2 ON parent.id = child2.id " + "JOIN secondary AS secondary_2 ON parent.id = " + "secondary_2.left_id " + "JOIN (parent AS parent_1 JOIN child1 AS child1_1 " + "ON parent_1.id = child1_1.id) ON parent_1.id = " + "secondary_2.right_id " + "WHERE parent_1.id = secondary_1.right_id " + "AND :param_1 = secondary_1.left_id", + ) + + # non aliased version self.assert_compile( sess.query(Child2) - .join(Child2.right_children) - .filter(Child1.left_child2 == c22) + .join(Child2.right_children.of_type(c1)) + .filter(c1.left_child2 == c22) .statement, "SELECT child2.id, parent.id AS id_1, parent.cls " "FROM secondary AS secondary_1, parent " @@ -2649,7 +2759,9 @@ class MultipleAdaptUsesEntityOverTableTest( def test_two_joins_adaption(self): a, c, d = self.tables.a, self.tables.c, self.tables.d - q = self._two_join_fixture()._compile_state() + + with _aliased_join_warning("C->c"), _aliased_join_warning("D->d"): + q = self._two_join_fixture()._compile_state() btoc = q.from_clauses[0].left @@ -2678,15 +2790,18 @@ class MultipleAdaptUsesEntityOverTableTest( def test_two_joins_sql(self): q = self._two_join_fixture() - self.assert_compile( - q, - "SELECT a.name AS a_name, a_1.name AS a_1_name, " - "a_2.name AS a_2_name " - "FROM a JOIN b ON a.id = b.id JOIN " - "(a AS a_1 JOIN c AS c_1 ON a_1.id = c_1.id) ON c_1.bid = b.id " - "JOIN (a AS a_2 JOIN d AS d_1 ON a_2.id = d_1.id) " - "ON d_1.cid = c_1.id", - ) + + with _aliased_join_warning("C->c"), _aliased_join_warning("D->d"): + self.assert_compile( + q, + "SELECT a.name AS a_name, a_1.name AS a_1_name, " + "a_2.name AS a_2_name " + "FROM a JOIN b ON a.id = b.id JOIN " + "(a AS a_1 JOIN c AS c_1 ON a_1.id = c_1.id) " + "ON c_1.bid = b.id " + "JOIN (a AS a_2 JOIN d AS d_1 ON a_2.id = d_1.id) " + "ON d_1.cid = c_1.id", + ) class SameNameOnJoined(fixtures.MappedTest): @@ -2810,33 +2925,45 @@ class BetweenSubclassJoinWExtraJoinedLoad( backref=backref("last_seen", lazy=False), ) - def test_query(self): + @testing.combinations((True,), (False,), argnames="autoalias") + def test_query_auto(self, autoalias): Engineer, Manager = self.classes("Engineer", "Manager") sess = fixture_session() - # eager join is both from Enginer->LastSeen as well as - # Manager->LastSeen. In the case of Manager->LastSeen, - # Manager is internally aliased, and comes to JoinedEagerLoader - # with no "parent" entity but an adapter. - q = sess.query(Engineer, Manager).join(Engineer.manager) - self.assert_compile( - q, - "SELECT people.type AS people_type, engineers.id AS engineers_id, " - "people.id AS people_id, " - "engineers.primary_language AS engineers_primary_language, " - "engineers.manager_id AS engineers_manager_id, " - "people_1.type AS people_1_type, managers_1.id AS managers_1_id, " - "people_1.id AS people_1_id, seen_1.id AS seen_1_id, " - "seen_1.timestamp AS seen_1_timestamp, seen_2.id AS seen_2_id, " - "seen_2.timestamp AS seen_2_timestamp " - "FROM people JOIN engineers ON people.id = engineers.id " - "JOIN (people AS people_1 JOIN managers AS managers_1 " - "ON people_1.id = managers_1.id) " - "ON managers_1.id = engineers.manager_id LEFT OUTER JOIN " - "seen AS seen_1 ON people.id = seen_1.id LEFT OUTER JOIN " - "seen AS seen_2 ON people_1.id = seen_2.id", - ) + if autoalias: + # eager join is both from Enginer->LastSeen as well as + # Manager->LastSeen. In the case of Manager->LastSeen, + # Manager is internally aliased, and comes to JoinedEagerLoader + # with no "parent" entity but an adapter. + q = sess.query(Engineer, Manager).join(Engineer.manager) + else: + m1 = aliased(Manager, flat=True) + q = sess.query(Engineer, m1).join(Engineer.manager.of_type(m1)) + + with _aliased_join_warning( + "Manager->managers" + ) if autoalias else util.nullcontext(): + self.assert_compile( + q, + "SELECT people.type AS people_type, engineers.id AS " + "engineers_id, " + "people.id AS people_id, " + "engineers.primary_language AS engineers_primary_language, " + "engineers.manager_id AS engineers_manager_id, " + "people_1.type AS people_1_type, " + "managers_1.id AS managers_1_id, " + "people_1.id AS people_1_id, seen_1.id AS seen_1_id, " + "seen_1.timestamp AS seen_1_timestamp, " + "seen_2.id AS seen_2_id, " + "seen_2.timestamp AS seen_2_timestamp " + "FROM people JOIN engineers ON people.id = engineers.id " + "JOIN (people AS people_1 JOIN managers AS managers_1 " + "ON people_1.id = managers_1.id) " + "ON managers_1.id = engineers.manager_id LEFT OUTER JOIN " + "seen AS seen_1 ON people.id = seen_1.id LEFT OUTER JOIN " + "seen AS seen_2 ON people_1.id = seen_2.id", + ) class M2ODontLoadSiblingTest(fixtures.DeclarativeMappedTest): diff --git a/test/orm/inheritance/test_single.py b/test/orm/inheritance/test_single.py index 32cc570204..1976dab060 100644 --- a/test/orm/inheritance/test_single.py +++ b/test/orm/inheritance/test_single.py @@ -10,6 +10,7 @@ from sqlalchemy import select from sqlalchemy import String from sqlalchemy import testing from sqlalchemy import true +from sqlalchemy import util from sqlalchemy.orm import aliased from sqlalchemy.orm import Bundle from sqlalchemy.orm import joinedload @@ -28,6 +29,13 @@ from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table +def _aliased_join_warning(arg): + return testing.expect_warnings( + "An alias is being generated automatically against joined entity " + "mapped class %s due to overlapping tables" % (arg,) + ) + + class SingleInheritanceTest(testing.AssertsCompiledSQL, fixtures.MappedTest): __dialect__ = "default" @@ -1772,24 +1780,32 @@ class SingleFromPolySelectableTest( "AS anon_1 WHERE anon_1.employee_type IN ([POSTCOMPILE_type_1])", ) - def test_single_inh_subclass_join_joined_inh_subclass(self): + @testing.combinations((True,), (False,), argnames="autoalias") + def test_single_inh_subclass_join_joined_inh_subclass(self, autoalias): Boss, Engineer = self.classes("Boss", "Engineer") s = fixture_session() - q = s.query(Boss).join(Engineer, Engineer.manager_id == Boss.id) + if autoalias: + q = s.query(Boss).join(Engineer, Engineer.manager_id == Boss.id) + else: + e1 = aliased(Engineer, flat=True) + q = s.query(Boss).join(e1, e1.manager_id == Boss.id) - self.assert_compile( - q, - "SELECT manager.id AS manager_id, employee.id AS employee_id, " - "employee.name AS employee_name, " - "employee.type AS employee_type, " - "manager.manager_data AS manager_manager_data " - "FROM employee JOIN manager ON employee.id = manager.id " - "JOIN (employee AS employee_1 JOIN engineer AS engineer_1 " - "ON employee_1.id = engineer_1.id) " - "ON engineer_1.manager_id = manager.id " - "WHERE employee.type IN ([POSTCOMPILE_type_1])", - ) + with _aliased_join_warning( + "Engineer->engineer" + ) if autoalias else util.nullcontext(): + self.assert_compile( + q, + "SELECT manager.id AS manager_id, employee.id AS employee_id, " + "employee.name AS employee_name, " + "employee.type AS employee_type, " + "manager.manager_data AS manager_manager_data " + "FROM employee JOIN manager ON employee.id = manager.id " + "JOIN (employee AS employee_1 JOIN engineer AS engineer_1 " + "ON employee_1.id = engineer_1.id) " + "ON engineer_1.manager_id = manager.id " + "WHERE employee.type IN ([POSTCOMPILE_type_1])", + ) def test_single_inh_subclass_join_wpoly_joined_inh_subclass(self): Boss = self.classes.Boss @@ -1828,25 +1844,35 @@ class SingleFromPolySelectableTest( "WHERE employee.type IN ([POSTCOMPILE_type_1])", ) - def test_joined_inh_subclass_join_single_inh_subclass(self): + @testing.combinations((True,), (False,), argnames="autoalias") + def test_joined_inh_subclass_join_single_inh_subclass(self, autoalias): Engineer = self.classes.Engineer Boss = self.classes.Boss s = fixture_session() - q = s.query(Engineer).join(Boss, Engineer.manager_id == Boss.id) + if autoalias: + q = s.query(Engineer).join(Boss, Engineer.manager_id == Boss.id) + else: + b1 = aliased(Boss, flat=True) + q = s.query(Engineer).join(b1, Engineer.manager_id == b1.id) - self.assert_compile( - q, - "SELECT engineer.id AS engineer_id, employee.id AS employee_id, " - "employee.name AS employee_name, employee.type AS employee_type, " - "engineer.engineer_info AS engineer_engineer_info, " - "engineer.manager_id AS engineer_manager_id " - "FROM employee JOIN engineer ON employee.id = engineer.id " - "JOIN (employee AS employee_1 JOIN manager AS manager_1 " - "ON employee_1.id = manager_1.id) " - "ON engineer.manager_id = manager_1.id " - "AND employee_1.type IN ([POSTCOMPILE_type_1])", - ) + with _aliased_join_warning( + "Boss->manager" + ) if autoalias else util.nullcontext(): + self.assert_compile( + q, + "SELECT engineer.id AS engineer_id, " + "employee.id AS employee_id, " + "employee.name AS employee_name, " + "employee.type AS employee_type, " + "engineer.engineer_info AS engineer_engineer_info, " + "engineer.manager_id AS engineer_manager_id " + "FROM employee JOIN engineer ON employee.id = engineer.id " + "JOIN (employee AS employee_1 JOIN manager AS manager_1 " + "ON employee_1.id = manager_1.id) " + "ON engineer.manager_id = manager_1.id " + "AND employee_1.type IN ([POSTCOMPILE_type_1])", + ) class EagerDefaultEvalTest(fixtures.DeclarativeMappedTest): diff --git a/test/orm/test_deprecations.py b/test/orm/test_deprecations.py index baa1f44cfb..95ed09a749 100644 --- a/test/orm/test_deprecations.py +++ b/test/orm/test_deprecations.py @@ -99,6 +99,20 @@ join_tuple_form = ( ) +def _aliased_join_warning(arg=None): + return testing.expect_warnings( + "An alias is being generated automatically against joined entity " + "mapped class " + (arg if arg else "") + ) + + +def _aliased_join_deprecation(arg=None): + return testing.expect_deprecated( + "An alias is being generated automatically against joined entity " + "mapped class " + (arg if arg else "") + ) + + class DeprecatedQueryTest(_fixtures.FixtureTest, AssertsCompiledSQL): __dialect__ = "default" @@ -824,7 +838,7 @@ class DeprecatedQueryTest(_fixtures.FixtureTest, AssertsCompiledSQL): oalias = orders.select() - with self._expect_implicit_subquery(): + with self._expect_implicit_subquery(), _aliased_join_warning(): self.assert_compile( sess.query(User) .join(oalias, User.orders) @@ -2494,7 +2508,11 @@ class DeprecatedInhTest(_poly_fixtures._Polymorphic): sess = fixture_session() mach_alias = machines.select() - with DeprecatedQueryTest._expect_implicit_subquery(): + + # note python 2 does not allow parens here; reformat in py3 only + with DeprecatedQueryTest._expect_implicit_subquery(), _aliased_join_warning( # noqa E501 + "Person->people" + ): self.assert_compile( sess.query(Company) .join(people.join(engineers), Company.employees) @@ -4827,19 +4845,20 @@ class AliasFromCorrectLeftTest( with testing.expect_deprecated_20(join_strings_dep): q = s.query(B).join(B.a_list, "x_list").filter(X.name == "x1") - self.assert_compile( - q, - "SELECT object.type AS object_type, b.id AS b_id, " - "object.id AS object_id, object.name AS object_name " - "FROM object JOIN b ON object.id = b.id " - "JOIN a_b_association AS a_b_association_1 " - "ON b.id = a_b_association_1.b_id " - "JOIN (" - "object AS object_1 " - "JOIN a AS a_1 ON object_1.id = a_1.id" - ") ON a_1.id = a_b_association_1.a_id " - "JOIN x ON object_1.id = x.obj_id WHERE x.name = :name_1", - ) + with _aliased_join_warning(): + self.assert_compile( + q, + "SELECT object.type AS object_type, b.id AS b_id, " + "object.id AS object_id, object.name AS object_name " + "FROM object JOIN b ON object.id = b.id " + "JOIN a_b_association AS a_b_association_1 " + "ON b.id = a_b_association_1.b_id " + "JOIN (" + "object AS object_1 " + "JOIN a AS a_1 ON object_1.id = a_1.id" + ") ON a_1.id = a_b_association_1.a_id " + "JOIN x ON object_1.id = x.obj_id WHERE x.name = :name_1", + ) def test_join_prop_to_prop(self): A, B, X = self.classes("A", "B", "X") @@ -4851,19 +4870,20 @@ class AliasFromCorrectLeftTest( with testing.expect_deprecated_20(join_chain_dep): q = s.query(B).join(B.a_list, A.x_list).filter(X.name == "x1") - self.assert_compile( - q, - "SELECT object.type AS object_type, b.id AS b_id, " - "object.id AS object_id, object.name AS object_name " - "FROM object JOIN b ON object.id = b.id " - "JOIN a_b_association AS a_b_association_1 " - "ON b.id = a_b_association_1.b_id " - "JOIN (" - "object AS object_1 " - "JOIN a AS a_1 ON object_1.id = a_1.id" - ") ON a_1.id = a_b_association_1.a_id " - "JOIN x ON object_1.id = x.obj_id WHERE x.name = :name_1", - ) + with _aliased_join_warning(): + self.assert_compile( + q, + "SELECT object.type AS object_type, b.id AS b_id, " + "object.id AS object_id, object.name AS object_name " + "FROM object JOIN b ON object.id = b.id " + "JOIN a_b_association AS a_b_association_1 " + "ON b.id = a_b_association_1.b_id " + "JOIN (" + "object AS object_1 " + "JOIN a AS a_1 ON object_1.id = a_1.id" + ") ON a_1.id = a_b_association_1.a_id " + "JOIN x ON object_1.id = x.obj_id WHERE x.name = :name_1", + ) class SelfReferentialTest(fixtures.MappedTest, AssertsCompiledSQL): @@ -5146,9 +5166,38 @@ class SelfReferentialTest(fixtures.MappedTest, AssertsCompiledSQL): ) -class InheritedJoinTest(_poly_fixtures._Polymorphic, AssertsCompiledSQL): +class InheritedJoinTest( + fixtures.NoCache, + _poly_fixtures._Polymorphic, + _poly_fixtures._PolymorphicFixtureBase, + AssertsCompiledSQL, +): run_setup_mappers = "once" + def test_join_to_selectable(self): + people, Company, engineers, Engineer = ( + self.tables.people, + self.classes.Company, + self.tables.engineers, + self.classes.Engineer, + ) + + sess = fixture_session() + + with _aliased_join_deprecation(): + self.assert_compile( + sess.query(Company) + .join(people.join(engineers), Company.employees) + .filter(Engineer.name == "dilbert"), + "SELECT companies.company_id AS companies_company_id, " + "companies.name AS companies_name " + "FROM companies JOIN (people " + "JOIN engineers ON people.person_id = " + "engineers.person_id) ON companies.company_id = " + "people.company_id WHERE people.name = :name_1", + use_default_dialect=True, + ) + def test_multiple_adaption(self): """test that multiple filter() adapters get chained together " and work correctly within a multiple-entry join().""" @@ -5166,7 +5215,9 @@ class InheritedJoinTest(_poly_fixtures._Polymorphic, AssertsCompiledSQL): mach_alias = aliased(Machine, machines.select().subquery()) - with testing.expect_deprecated_20(join_aliased_dep): + with testing.expect_deprecated_20( + join_aliased_dep + ), _aliased_join_deprecation(): self.assert_compile( sess.query(Company) .join(people.join(engineers), Company.employees) @@ -5274,6 +5325,95 @@ class InheritedJoinTest(_poly_fixtures._Polymorphic, AssertsCompiledSQL): ): str(q) + def test_join_to_subclass_selectable_auto_alias(self): + Company, Engineer = self.classes("Company", "Engineer") + people, engineers = self.tables("people", "engineers") + + sess = fixture_session() + + with _aliased_join_deprecation(): + eq_( + sess.query(Company) + .join(people.join(engineers), "employees") + .filter(Engineer.primary_language == "java") + .all(), + [self.c1], + ) + + # occurs for 2.0 style query also + with _aliased_join_deprecation(): + stmt = ( + select(Company) + .join(people.join(engineers), Company.employees) + .filter(Engineer.primary_language == "java") + ) + results = sess.scalars(stmt) + eq_(results.all(), [self.c1]) + + def test_join_to_subclass_two(self): + Company, Engineer = self.classes("Company", "Engineer") + people, engineers = self.tables("people", "engineers") + + sess = fixture_session() + + with _aliased_join_deprecation(): + eq_( + sess.query(Company) + .join(people.join(engineers), "employees") + .filter(Engineer.primary_language == "java") + .all(), + [self.c1], + ) + + def test_join_to_subclass_six_selectable_auto_alias(self): + Company, Engineer = self.classes("Company", "Engineer") + people, engineers = self.tables("people", "engineers") + + sess = fixture_session() + + with _aliased_join_deprecation(): + eq_( + sess.query(Company) + .join(people.join(engineers), "employees") + .join(Engineer.machines) + .all(), + [self.c1, self.c2], + ) + + def test_join_to_subclass_six_point_five_selectable_auto_alias(self): + Company, Engineer = self.classes("Company", "Engineer") + people, engineers = self.tables("people", "engineers") + + sess = fixture_session() + + with _aliased_join_deprecation(): + eq_( + sess.query(Company) + .join(people.join(engineers), "employees") + .join(Engineer.machines) + .filter(Engineer.name == "dilbert") + .all(), + [self.c1], + ) + + def test_join_to_subclass_seven_selectable_auto_alias(self): + Company, Engineer, Machine = self.classes( + "Company", "Engineer", "Machine" + ) + people, engineers = self.tables("people", "engineers") + + sess = fixture_session() + + with _aliased_join_deprecation(): + eq_( + sess.query(Company) + .join(people.join(engineers), "employees") + .join(Engineer.machines) + .filter(Machine.name.ilike("%thinkpad%")) + .all(), + [self.c1], + ) + class JoinFromSelectableTest(fixtures.MappedTest, AssertsCompiledSQL): __dialect__ = "default" @@ -5498,3 +5638,128 @@ class DeprecationScopedSessionTest(fixtures.MappedTest): ): Session() Session.remove() + + +@testing.combinations( + ( + "inline", + True, + ), + ( + "separate", + False, + ), + argnames="inline", + id_="sa", +) +@testing.combinations( + ( + "string", + True, + ), + ( + "literal", + False, + ), + argnames="stringbased", + id_="sa", +) +class ExplicitJoinTest(fixtures.MappedTest): + @classmethod + def define_tables(cls, metadata): + global User, Address + Base = declarative_base(metadata=metadata) + + class User(Base, fixtures.ComparableEntity): + + __tablename__ = "users" + id = Column( + Integer, primary_key=True, test_needs_autoincrement=True + ) + name = Column(String(50)) + + class Address(Base, fixtures.ComparableEntity): + + __tablename__ = "addresses" + id = Column( + Integer, primary_key=True, test_needs_autoincrement=True + ) + email = Column(String(50)) + user_id = Column(Integer, ForeignKey("users.id")) + if cls.inline: + if cls.stringbased: + user = relationship( + "User", + primaryjoin="User.id==Address.user_id", + backref="addresses", + ) + else: + user = relationship( + User, + primaryjoin=User.id == user_id, + backref="addresses", + ) + + if not cls.inline: + configure_mappers() + if cls.stringbased: + Address.user = relationship( + "User", + primaryjoin="User.id==Address.user_id", + backref="addresses", + ) + else: + Address.user = relationship( + User, + primaryjoin=User.id == Address.user_id, + backref="addresses", + ) + + @classmethod + def insert_data(cls, connection): + params = [ + dict(list(zip(("id", "name"), column_values))) + for column_values in [ + (7, "jack"), + (8, "ed"), + (9, "fred"), + (10, "chuck"), + ] + ] + + connection.execute(User.__table__.insert(), params) + connection.execute( + Address.__table__.insert(), + [ + dict(list(zip(("id", "user_id", "email"), column_values))) + for column_values in [ + (1, 7, "jack@bean.com"), + (2, 8, "ed@wood.com"), + (3, 8, "ed@bettyboop.com"), + (4, 8, "ed@lala.com"), + (5, 9, "fred@fred.com"), + ] + ], + ) + + def test_aliased_join(self): + + # this query will screw up if the aliasing enabled in + # query.join() gets applied to the right half of the join + # condition inside the any(). the join condition inside of + # any() comes from the "primaryjoin" of the relationship, + # and should not be annotated with _orm_adapt. + # PropertyLoader.Comparator will annotate the left side with + # _orm_adapt, though. + + sess = fixture_session() + + with testing.expect_deprecated_20(join_aliased_dep): + eq_( + sess.query(User) + .join(User.addresses, aliased=True) + .filter(Address.email == "ed@wood.com") + .filter(User.addresses.any(Address.email == "jack@bean.com")) + .all(), + [], + ) diff --git a/test/orm/test_froms.py b/test/orm/test_froms.py index f7ee1a94c1..8ddb9ee4c0 100644 --- a/test/orm/test_froms.py +++ b/test/orm/test_froms.py @@ -2519,7 +2519,7 @@ class MixedEntitiesTest(QueryTest, AssertsCompiledSQL): .add_columns( func.count(adalias.c.id), ("Name:" + users.c.name) ) - .outerjoin(adalias, "addresses") + .outerjoin(adalias) .group_by(users) .order_by(users.c.id) ) @@ -2582,7 +2582,7 @@ class MixedEntitiesTest(QueryTest, AssertsCompiledSQL): .add_columns( func.count(adalias.c.id), ("Name:" + users.c.name) ) - .outerjoin(adalias, "addresses") + .outerjoin(adalias) .group_by(users) .order_by(users.c.id) ) diff --git a/test/orm/test_joins.py b/test/orm/test_joins.py index 5391892830..d785c2ba03 100644 --- a/test/orm/test_joins.py +++ b/test/orm/test_joins.py @@ -61,29 +61,6 @@ class InheritedJoinTest(InheritedTest, AssertsCompiledSQL): use_default_dialect=True, ) - def test_join_to_selectable(self): - people, Company, engineers, Engineer = ( - self.tables.people, - self.classes.Company, - self.tables.engineers, - self.classes.Engineer, - ) - - sess = fixture_session() - - self.assert_compile( - sess.query(Company) - .join(people.join(engineers), Company.employees) - .filter(Engineer.name == "dilbert"), - "SELECT companies.company_id AS companies_company_id, " - "companies.name AS companies_name " - "FROM companies JOIN (people " - "JOIN engineers ON people.person_id = " - "engineers.person_id) ON companies.company_id = " - "people.company_id WHERE people.name = :name_1", - use_default_dialect=True, - ) - def test_force_via_select_from(self): Company, Engineer = self.classes.Company, self.classes.Engineer @@ -188,22 +165,30 @@ class InheritedJoinTest(InheritedTest, AssertsCompiledSQL): .join(Company.employees.of_type(Boss)) ) - self.assert_compile( - q, - "SELECT companies.company_id AS companies_company_id, " - "companies.name AS companies_name FROM companies " - "JOIN (people JOIN engineers " - "ON people.person_id = engineers.person_id) " - "ON companies.company_id = people.company_id " - "JOIN (people AS people_1 JOIN managers AS managers_1 " - "ON people_1.person_id = managers_1.person_id) " - "ON companies.company_id = people_1.company_id " - "JOIN (people AS people_2 JOIN managers AS managers_2 " - "ON people_2.person_id = managers_2.person_id JOIN boss AS boss_1 " - "ON managers_2.person_id = boss_1.boss_id) " - "ON companies.company_id = people_2.company_id", - use_default_dialect=True, - ) + with testing.expect_warnings( + "An alias is being generated automatically against joined entity " + "mapped class Manager->managers due to overlapping", + "An alias is being generated automatically against joined entity " + "mapped class Boss->boss due to overlapping", + raise_on_any_unexpected=True, + ): + self.assert_compile( + q, + "SELECT companies.company_id AS companies_company_id, " + "companies.name AS companies_name FROM companies " + "JOIN (people JOIN engineers " + "ON people.person_id = engineers.person_id) " + "ON companies.company_id = people.company_id " + "JOIN (people AS people_1 JOIN managers AS managers_1 " + "ON people_1.person_id = managers_1.person_id) " + "ON companies.company_id = people_1.company_id " + "JOIN (people AS people_2 JOIN managers AS managers_2 " + "ON people_2.person_id = managers_2.person_id " + "JOIN boss AS boss_1 " + "ON managers_2.person_id = boss_1.boss_id) " + "ON companies.company_id = people_2.company_id", + use_default_dialect=True, + ) class JoinOnSynonymTest(_fixtures.FixtureTest, AssertsCompiledSQL): diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index d8da3a1f63..3ba4e90db3 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -798,7 +798,8 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): relationship(Address), ) - def test_add_column_prop_deannotate(self): + @testing.combinations((True,), (False,)) + def test_add_column_prop_deannotate(self, autoalias): User, users = self.classes.User, self.tables.users Address, addresses = self.classes.Address, self.tables.addresses @@ -817,22 +818,47 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): # needs to be deannotated m.add_property("x", column_property(User.name + "name")) s = fixture_session() - q = s.query(m2).select_from(Address).join(Address.foo) - self.assert_compile( - q, - "SELECT " - "addresses_1.id AS addresses_1_id, " - "users_1.id AS users_1_id, " - "users_1.name AS users_1_name, " - "addresses_1.user_id AS addresses_1_user_id, " - "addresses_1.email_address AS " - "addresses_1_email_address, " - "users_1.name || :name_1 AS anon_1 " - "FROM addresses JOIN (users AS users_1 JOIN addresses " - "AS addresses_1 ON users_1.id = " - "addresses_1.user_id) ON " - "users_1.id = addresses.user_id", - ) + + if autoalias: + q = s.query(m2).select_from(Address).join(Address.foo) + + with testing.expect_warnings( + "An alias is being generated automatically against joined " + "entity mapped class SubUser" + ): + self.assert_compile( + q, + "SELECT " + "addresses_1.id AS addresses_1_id, " + "users_1.id AS users_1_id, " + "users_1.name AS users_1_name, " + "addresses_1.user_id AS addresses_1_user_id, " + "addresses_1.email_address AS " + "addresses_1_email_address, " + "users_1.name || :name_1 AS anon_1 " + "FROM addresses JOIN (users AS users_1 JOIN addresses " + "AS addresses_1 ON users_1.id = " + "addresses_1.user_id) ON " + "users_1.id = addresses.user_id", + ) + else: + m3 = aliased(m2, flat=True) + q = s.query(m3).select_from(Address).join(Address.foo.of_type(m3)) + self.assert_compile( + q, + "SELECT " + "addresses_1.id AS addresses_1_id, " + "users_1.id AS users_1_id, " + "users_1.name AS users_1_name, " + "addresses_1.user_id AS addresses_1_user_id, " + "addresses_1.email_address AS " + "addresses_1_email_address, " + "users_1.name || :name_1 AS anon_1 " + "FROM addresses JOIN (users AS users_1 JOIN addresses " + "AS addresses_1 ON users_1.id = " + "addresses_1.user_id) ON " + "users_1.id = addresses.user_id", + ) def test_column_prop_deannotate(self): """test that column property deannotates, -- 2.47.3