From 8b8a4c373e1cec2203f73b1772704b9ded75c1fe Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 5 Mar 2021 16:02:38 -0500 Subject: [PATCH] Ensure all Query statements compile w/ orm, fix test harness Fixed regression where :meth:`_orm.Query.join` would produce no effect if the query itself as well as the join target were against a :class:`_schema.Table` object, rather than a mapped class. This was part of a more systemic issue where the legacy ORM query compiler would not be correctly used from a :class:`_orm.Query` if the statement produced had not ORM entities present within it. Also repair the assert_compile() method, which was using Query._compile_state() which was bypassing the bug. A handful of ORM tests with Query objects and Core-only objects were actually failing if the default compilation path were used. Fixes: #6003 Change-Id: I97478b2d06bf6c01fe1f09ee118e1f2ac4c849bc --- doc/build/changelog/unreleased_14/6003.rst | 11 ++++ lib/sqlalchemy/orm/query.py | 10 +++ lib/sqlalchemy/testing/assertions.py | 8 +-- test/orm/test_joins.py | 76 +++++++++++++++++++++- 4 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/6003.rst diff --git a/doc/build/changelog/unreleased_14/6003.rst b/doc/build/changelog/unreleased_14/6003.rst new file mode 100644 index 0000000000..e26d6a86cb --- /dev/null +++ b/doc/build/changelog/unreleased_14/6003.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 6003 + + Fixed regression where :meth:`_orm.Query.join` would produce no effect if + the query itself as well as the join target were against a + :class:`_schema.Table` object, rather than a mapped class. This was part of + a more systemic issue where the legacy ORM query compiler would not be + correctly used from a :class:`_orm.Query` if the statement produced had not + ORM entities present within it. + diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index c444b557bb..d685ac83ed 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -440,6 +440,15 @@ class Query( ) stmt.__dict__.pop("session", None) + # ensure the ORM context is used to compile the statement, even + # if it has no ORM entities. This is so ORM-only things like + # _legacy_joins are picked up that wouldn't be picked up by the + # Core statement context + if "compile_state_plugin" not in stmt._propagate_attrs: + stmt._propagate_attrs = stmt._propagate_attrs.union( + {"compile_state_plugin": "orm", "plugin_subject": None} + ) + return stmt def subquery( @@ -2304,6 +2313,7 @@ class Query( "is deprecated and will be removed in SQLAlchemy 2.0. " "Please use individual join() calls per relationship." ) + self._legacy_setup_joins += joins_to_add self.__dict__.pop("_last_joined_entity", None) diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py index 81c74e7c16..289ac9a0a3 100644 --- a/lib/sqlalchemy/testing/assertions.py +++ b/lib/sqlalchemy/testing/assertions.py @@ -445,11 +445,9 @@ class AssertsCompiledSQL(object): from sqlalchemy import orm if isinstance(clause, orm.Query): - compile_state = clause._compile_state() - compile_state.statement._label_style = ( - LABEL_STYLE_TABLENAME_PLUS_COL - ) - clause = compile_state.statement + stmt = clause._statement_20() + stmt._label_style = LABEL_STYLE_TABLENAME_PLUS_COL + clause = stmt if compile_kwargs: kw["compile_kwargs"] = compile_kwargs diff --git a/test/orm/test_joins.py b/test/orm/test_joins.py index de5a5c92d5..6dc1d5cead 100644 --- a/test/orm/test_joins.py +++ b/test/orm/test_joins.py @@ -13,6 +13,7 @@ from sqlalchemy import or_ from sqlalchemy import select from sqlalchemy import String from sqlalchemy import Table +from sqlalchemy import testing from sqlalchemy import true from sqlalchemy.engine import default from sqlalchemy.orm import aliased @@ -1066,7 +1067,9 @@ class JoinTest(QueryTest, AssertsCompiledSQL): "ON addresses_1.id = dingalings.address_id", ) - def test_pure_expression_error(self): + def test_pure_expression(self): + # this was actually false-passing due to the assertions + # fixture not following the regular codepath for Query addresses, users = self.tables.addresses, self.tables.users sess = fixture_session() @@ -3063,3 +3066,74 @@ class JoinLateralTest(fixtures.MappedTest, AssertsCompiledSQL): "generate_series(:generate_series_1, anon_1.bookcase_shelves) " "AS anon_2 ON true", ) + + +class JoinRawTablesWLegacyTest(QueryTest, AssertsCompiledSQL): + """test issue 6003 where creating a legacy query with only Core elements + fails to accommodate for the ORM context thus producing a query + that ignores the "legacy" joins + + """ + + __dialect__ = "default" + + @testing.combinations( + ( + lambda sess, User, Address: sess.query(User).join(Address), + "SELECT users.id AS users_id, users.name AS users_name FROM " + "users JOIN addresses ON users.id = addresses.user_id", + ), + ( + lambda sess, user_table, address_table: sess.query( + user_table + ).join(address_table), + "SELECT users.id AS users_id, users.name AS users_name FROM " + "users JOIN addresses ON users.id = addresses.user_id", + ), + ( + lambda sess, User, Address, Order: sess.query(User) + .outerjoin(Order) + .join(Address), + "SELECT users.id AS users_id, users.name AS users_name FROM " + "users LEFT OUTER JOIN orders ON users.id = orders.user_id " + "JOIN addresses ON addresses.id = orders.address_id", + ), + ( + lambda sess, user_table, address_table, order_table: sess.query( + user_table + ) + .outerjoin(order_table) + .join(address_table), + "SELECT users.id AS users_id, users.name AS users_name FROM " + "users LEFT OUTER JOIN orders ON users.id = orders.user_id " + "JOIN addresses ON addresses.id = orders.address_id", + ), + ) + def test_join_render(self, spec, expected): + User, Address, Order = self.classes("User", "Address", "Order") + user_table, address_table, order_table = self.tables( + "users", "addresses", "orders" + ) + + sess = fixture_session() + + q = testing.resolve_lambda(spec, **locals()) + + self.assert_compile(q, expected) + + self.assert_compile( + q.set_label_style(LABEL_STYLE_TABLENAME_PLUS_COL).statement, + expected, + ) + + def test_core_round_trip(self): + user_table, address_table = self.tables("users", "addresses") + + sess = fixture_session() + + q = ( + sess.query(user_table) + .join(address_table) + .where(address_table.c.email_address.startswith("ed")) + ) + eq_(q.all(), [(8, "ed"), (8, "ed"), (8, "ed")]) -- 2.47.2