From d2cf7dcfe0cd7e9986376b6e7edd4b7d60108599 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 26 Oct 2021 11:28:42 -0400 Subject: [PATCH] dont pull filter_by from from_obj, entity is hopefully sufficient Fixed 1.4 regression where :meth:`_orm.Query.filter_by` would not function correctly on a :class:`_orm.Query` that was produced from :meth:`_orm.Query.union`, :meth:`_orm.Query.from_self` or similar. Fixes: #7239 Change-Id: I3a0c3fd71180b1bfb6bf855f436a29c729664082 --- doc/build/changelog/unreleased_14/7239.rst | 7 +++ lib/sqlalchemy/orm/query.py | 32 ++++++++++- test/orm/test_deprecations.py | 66 ++++++++++++++++++++++ test/orm/test_query.py | 41 ++++++++++++++ 4 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 doc/build/changelog/unreleased_14/7239.rst diff --git a/doc/build/changelog/unreleased_14/7239.rst b/doc/build/changelog/unreleased_14/7239.rst new file mode 100644 index 0000000000..14ef19118f --- /dev/null +++ b/doc/build/changelog/unreleased_14/7239.rst @@ -0,0 +1,7 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 7239 + + Fixed 1.4 regression where :meth:`_orm.Query.filter_by` would not function + correctly on a :class:`_orm.Query` that was produced from + :meth:`_orm.Query.union`, :meth:`_orm.Query.from_self` or similar. diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index fcb9dae57a..d48e34923a 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -1707,12 +1707,42 @@ class Query( return None def _filter_by_zero(self): + """for the filter_by() method, return the target entity for which + we will attempt to derive an expression from based on string name. + + """ if self._legacy_setup_joins: _last_joined_entity = self._last_joined_entity if _last_joined_entity is not None: return _last_joined_entity - if self._from_obj: + # discussion related to #7239 + # special check determines if we should try to derive attributes + # for filter_by() from the "from object", i.e., if the user + # called query.select_from(some selectable).filter_by(some_attr=value). + # We don't want to do that in the case that methods like + # from_self(), select_entity_from(), or a set op like union() were + # called; while these methods also place a + # selectable in the _from_obj collection, they also set up + # the _set_base_alias boolean which turns on the whole "adapt the + # entity to this selectable" thing, meaning the query still continues + # to construct itself in terms of the lead entity that was passed + # to query(), e.g. query(User).from_self() is still in terms of User, + # and not the subquery that from_self() created. This feature of + # "implicitly adapt all occurrences of entity X to some arbitrary + # subquery" is the main thing I am trying to do away with in 2.0 as + # users should now used aliased() for that, but I can't entirely get + # rid of it due to query.union() and other set ops relying upon it. + # + # compare this to the base Select()._filter_by_zero() which can + # just return self._from_obj[0] if present, because there is no + # "_set_base_alias" feature. + # + # IOW, this conditional essentially detects if + # "select_from(some_selectable)" has been called, as opposed to + # "select_entity_from()", "from_self()" + # or "union() / some_set_op()". + if self._from_obj and not self._compile_options._set_base_alias: return self._from_obj[0] return self._raw_columns[0] diff --git a/test/orm/test_deprecations.py b/test/orm/test_deprecations.py index b868c63fe1..c66259e478 100644 --- a/test/orm/test_deprecations.py +++ b/test/orm/test_deprecations.py @@ -10,6 +10,7 @@ from sqlalchemy import func from sqlalchemy import inspect from sqlalchemy import Integer from sqlalchemy import join +from sqlalchemy import LABEL_STYLE_TABLENAME_PLUS_COL from sqlalchemy import lateral from sqlalchemy import literal_column from sqlalchemy import MetaData @@ -79,6 +80,7 @@ from sqlalchemy.testing.mock import call from sqlalchemy.testing.mock import Mock from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table +from sqlalchemy.testing.util import resolve_lambda from sqlalchemy.util import pickle from . import _fixtures from .inheritance import _poly_fixtures @@ -1605,6 +1607,24 @@ class FromSelfTest(QueryTest, AssertsCompiledSQL): {}, ) + def test_basic_filter_by(self): + """test #7239""" + + User = self.classes.User + + s = fixture_session() + + with self._from_self_deprecated(): + q = s.query(User).from_self() + + self.assert_compile( + q.filter_by(id=5), + "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name " + "AS anon_1_users_name FROM (SELECT users.id AS users_id, " + "users.name AS users_name FROM users) AS anon_1 " + "WHERE anon_1.users_id = :id_1", + ) + def test_columns_augmented_distinct_on(self): User, Address = self.classes.User, self.classes.Address @@ -7328,6 +7348,52 @@ class SelectFromTest(QueryTest, AssertsCompiledSQL): "AS anon_1", ) + @testing.combinations( + ( + lambda users: users.select().where(users.c.id.in_([7, 8])), + "SELECT anon_1.id AS anon_1_id, anon_1.name AS anon_1_name " + "FROM (SELECT users.id AS id, users.name AS name " + "FROM users WHERE users.id IN ([POSTCOMPILE_id_1])) AS anon_1 " + "WHERE anon_1.name = :name_1", + ), + ( + lambda users: users.select() + .where(users.c.id.in_([7, 8])) + .set_label_style(LABEL_STYLE_TABLENAME_PLUS_COL), + "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name " + "AS anon_1_users_name FROM (SELECT users.id AS users_id, " + "users.name AS users_name FROM users " + "WHERE users.id IN ([POSTCOMPILE_id_1])) AS anon_1 " + "WHERE anon_1.users_name = :name_1", + ), + ( + lambda User, sess: sess.query(User).where(User.id.in_([7, 8])), + "SELECT anon_1.id AS anon_1_id, anon_1.name AS anon_1_name " + "FROM (SELECT users.id AS id, users.name AS name " + "FROM users WHERE users.id IN ([POSTCOMPILE_id_1])) AS anon_1 " + "WHERE anon_1.name = :name_1", + ), + ) + def test_filter_by(self, query_fn, expected): + """test #7239""" + + User = self.classes.User + sess = fixture_session() + + User, users = self.classes.User, self.tables.users + + self.mapper_registry.map_imperatively(User, users) + + sel = resolve_lambda(query_fn, User=User, users=users, sess=sess) + + sess = fixture_session() + + with assertions.expect_deprecated_20(sef_dep): + q = sess.query(User).select_entity_from(sel.subquery()) + + self.assert_compile(q.filter_by(name="ed"), expected) + eq_(q.filter_by(name="ed").all(), [User(name="ed")]) + def test_join_no_order_by(self): User, users = self.classes.User, self.tables.users diff --git a/test/orm/test_query.py b/test/orm/test_query.py index 2320124cdd..1971964b13 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -3619,6 +3619,47 @@ class FilterTest(QueryTest, AssertsCompiledSQL): "WHERE users.name = :name_1", ) + def test_filter_by_against_union_legacy(self): + """test #7239""" + + User = self.classes.User + sess = fixture_session() + + q = ( + sess.query(User) + .filter(User.id == 2) + .union(sess.query(User).filter(User.id == 5)) + ) + + self.assert_compile( + q.filter_by(id=5), + "SELECT anon_1.users_id AS anon_1_users_id, anon_1.users_name " + "AS anon_1_users_name FROM (SELECT users.id AS users_id, " + "users.name AS users_name FROM users WHERE users.id = :id_1 " + "UNION SELECT users.id AS users_id, users.name AS users_name " + "FROM users WHERE users.id = :id_2) AS anon_1 " + "WHERE anon_1.users_id = :id_3", + ) + + def test_filter_by_against_union_newstyle(self): + """test #7239""" + + User = self.classes.User + + u = union( + select(User).filter(User.id == 2), + select(User).filter(User.id == 5), + ) + ua = aliased(User, u.subquery()) + stmt = select(ua) + self.assert_compile( + stmt.filter_by(id=5), + "SELECT anon_1.id, anon_1.name FROM (SELECT users.id AS id, " + "users.name AS name FROM users WHERE users.id = :id_1 " + "UNION SELECT users.id AS id, users.name AS name FROM users " + "WHERE users.id = :id_2) AS anon_1 WHERE anon_1.id = :id_3", + ) + def test_filter_by_against_cast(self): """test #6414""" User = self.classes.User -- 2.47.3