diff options
24 files changed, 1081 insertions, 408 deletions
diff --git a/doc/build/changelog/unreleased_14/6974.rst b/doc/build/changelog/unreleased_14/6974.rst new file mode 100644 index 000000000..fd6b97009 --- /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 000000000..7a3f6d410 --- /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 a8e0939a4..da5262381 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 <self_referential>`. 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 04188232e..2b1d9ae89 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 e931c783c..2f1c02102 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 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): diff --git a/test/aaa_profiling/test_memusage.py b/test/aaa_profiling/test_memusage.py index 6462ab9f1..1c08c30a2 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 7de1811ae..727d48fca 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 ecc824391..4e2c465d8 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 cbf7b5042..084305a7c 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 1b4a367ac..1ca005bd0 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 de7723125..1a96ee011 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 b9f3ac01d..164b13f2e 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 32cc57020..1976dab06 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 baa1f44cf..95ed09a74 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 f7ee1a94c..8ddb9ee4c 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 539189283..d785c2ba0 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 d8da3a1f6..3ba4e90db 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, |