diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2021-05-01 08:41:09 -0400 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2021-05-01 11:54:21 -0400 |
commit | 5cde12d816b8b32719cee377677667150df07c48 (patch) | |
tree | 2bfe72c72617f617d80301d08e19a845211fad9f | |
parent | a47c158a9a3b1104698fc0bff47ca58d67cb9191 (diff) | |
download | sqlalchemy-5cde12d816b8b32719cee377677667150df07c48.tar.gz |
Support filter_by() from columns, functions, Core SQL
Fixed regression where :meth:`_orm.Query.filter_by` would not work if the
lead entity were a SQL function or other expression derived from the
primary entity in question, rather than a simple entity or column of that
entity. Additionally, improved the behavior of
:meth:`_sql.Select.filter_by` overall to work with column expressions even
in a non-ORM context.
Fixes: #6414
Change-Id: I316b5bf98293bec1ede08787f6181dd14be85419
-rw-r--r-- | doc/build/changelog/unreleased_14/6414.rst | 10 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/base.py | 20 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/elements.py | 14 | ||||
-rw-r--r-- | lib/sqlalchemy/sql/functions.py | 11 | ||||
-rw-r--r-- | test/orm/test_query.py | 57 | ||||
-rw-r--r-- | test/sql/test_select.py | 65 |
6 files changed, 175 insertions, 2 deletions
diff --git a/doc/build/changelog/unreleased_14/6414.rst b/doc/build/changelog/unreleased_14/6414.rst new file mode 100644 index 000000000..353f5265f --- /dev/null +++ b/doc/build/changelog/unreleased_14/6414.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 6414 + + Fixed regression where :meth:`_orm.Query.filter_by` would not work if the + lead entity were a SQL function or other expression derived from the + primary entity in question, rather than a simple entity or column of that + entity. Additionally, improved the behavior of + :meth:`_sql.Select.filter_by` overall to work with column expressions even + in a non-ORM context. diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py index d9f05e823..6d65d9061 100644 --- a/lib/sqlalchemy/sql/base.py +++ b/lib/sqlalchemy/sql/base.py @@ -15,6 +15,7 @@ import operator import re from . import roles +from . import visitors from .traversals import HasCacheKey # noqa from .traversals import HasCopyInternals # noqa from .traversals import MemoizedHasCacheKey # noqa @@ -1575,6 +1576,23 @@ def _bind_or_error(schemaitem, msg=None): return bind +def _entity_namespace(entity): + """Return the nearest .entity_namespace for the given entity. + + If not immediately available, does an iterate to find a sub-element + that has one, if any. + + """ + try: + return entity.entity_namespace + except AttributeError: + for elem in visitors.iterate(entity): + if hasattr(elem, "entity_namespace"): + return elem.entity_namespace + else: + raise + + def _entity_namespace_key(entity, key): """Return an entry from an entity_namespace. @@ -1584,8 +1602,8 @@ def _entity_namespace_key(entity, key): """ - ns = entity.entity_namespace try: + ns = _entity_namespace(entity) return getattr(ns, key) except AttributeError as err: util.raise_( diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index e27b97802..416a4e82e 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -300,6 +300,13 @@ class ClauseElement( f = f._is_clone_of return s + @property + def entity_namespace(self): + raise AttributeError( + "This SQL expression has no entity namespace " + "with which to filter from." + ) + def __getstate__(self): d = self.__dict__.copy() d.pop("_is_clone_of", None) @@ -4664,6 +4671,13 @@ class ColumnClause( # expect the columns of tables and subqueries to be leaf nodes. return [] + @property + def entity_namespace(self): + if self.table is not None: + return self.table.entity_namespace + else: + return super(ColumnClause, self).entity_namespace + @HasMemoized.memoized_attribute def _from_objects(self): t = self.table diff --git a/lib/sqlalchemy/sql/functions.py b/lib/sqlalchemy/sql/functions.py index 02ed55100..0aa870ce4 100644 --- a/lib/sqlalchemy/sql/functions.py +++ b/lib/sqlalchemy/sql/functions.py @@ -15,6 +15,7 @@ from . import roles from . import schema from . import sqltypes from . import util as sqlutil +from .base import _entity_namespace from .base import ColumnCollection from .base import Executable from .base import Generative @@ -618,6 +619,16 @@ class FunctionElement(Executable, ColumnElement, FromClause, Generative): else: return super(FunctionElement, self).self_group(against=against) + @property + def entity_namespace(self): + """overrides FromClause.entity_namespace as functions are generally + column expressions and not FromClauses. + + """ + # ideally functions would not be fromclauses but we failed to make + # this adjustment in 1.4 + return _entity_namespace(self.clause_expr) + class FunctionAsBinary(BinaryExpression): _traverse_internals = [ diff --git a/test/orm/test_query.py b/test/orm/test_query.py index d1723bcf1..d26f94bb8 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -3357,6 +3357,63 @@ class FilterTest(QueryTest, AssertsCompiledSQL): checkparams={"email_address_1": "ed@ed.com", "name_1": "ed"}, ) + def test_filter_by_against_function(self): + """test #6414 + + this is related to #6401 where the fact that Function is a + FromClause, an architectural mistake that we unfortunately did not + fix, is confusing the use of entity_namespace etc. + + """ + User = self.classes.User + sess = fixture_session() + + q1 = sess.query(func.count(User.id)).filter_by(name="ed") + + self.assert_compile( + q1, + "SELECT count(users.id) AS count_1 FROM users " + "WHERE users.name = :name_1", + ) + + def test_filter_by_against_cast(self): + """test #6414""" + User = self.classes.User + sess = fixture_session() + + q1 = sess.query(cast(User.id, Integer)).filter_by(name="ed") + + self.assert_compile( + q1, + "SELECT CAST(users.id AS INTEGER) AS users_id FROM users " + "WHERE users.name = :name_1", + ) + + def test_filter_by_against_binary(self): + """test #6414""" + User = self.classes.User + sess = fixture_session() + + q1 = sess.query(User.id == 5).filter_by(name="ed") + + self.assert_compile( + q1, + "SELECT users.id = :id_1 AS anon_1 FROM users " + "WHERE users.name = :name_1", + ) + + def test_filter_by_against_label(self): + """test #6414""" + User = self.classes.User + sess = fixture_session() + + q1 = sess.query(User.id.label("foo")).filter_by(name="ed") + + self.assert_compile( + q1, + "SELECT users.id AS foo FROM users " "WHERE users.name = :name_1", + ) + def test_empty_filters(self): User = self.classes.User sess = fixture_session() diff --git a/test/sql/test_select.py b/test/sql/test_select.py index 1dfb4cd19..f9f1acfa0 100644 --- a/test/sql/test_select.py +++ b/test/sql/test_select.py @@ -1,6 +1,8 @@ +from sqlalchemy import cast from sqlalchemy import Column from sqlalchemy import exc from sqlalchemy import ForeignKey +from sqlalchemy import func from sqlalchemy import Integer from sqlalchemy import MetaData from sqlalchemy import select @@ -13,6 +15,7 @@ from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import fixtures + table1 = table( "mytable", column("myid", Integer), @@ -296,7 +299,59 @@ class FutureSelectTest(fixtures.TestBase, AssertsCompiledSQL): checkparams={"data_1": "p1", "data_2": "c1", "otherid_1": 5}, ) - def test_filter_by_no_property(self): + def test_filter_by_from_col(self): + stmt = select(table1.c.myid).filter_by(name="foo") + self.assert_compile( + stmt, + "SELECT mytable.myid FROM mytable WHERE mytable.name = :name_1", + ) + + def test_filter_by_from_func(self): + """test #6414""" + stmt = select(func.count(table1.c.myid)).filter_by(name="foo") + self.assert_compile( + stmt, + "SELECT count(mytable.myid) AS count_1 " + "FROM mytable WHERE mytable.name = :name_1", + ) + + def test_filter_by_from_func_not_the_first_arg(self): + """test #6414""" + stmt = select(func.bar(True, table1.c.myid)).filter_by(name="foo") + self.assert_compile( + stmt, + "SELECT bar(:bar_2, mytable.myid) AS bar_1 " + "FROM mytable WHERE mytable.name = :name_1", + ) + + def test_filter_by_from_cast(self): + """test #6414""" + stmt = select(cast(table1.c.myid, Integer)).filter_by(name="foo") + self.assert_compile( + stmt, + "SELECT CAST(mytable.myid AS INTEGER) AS myid " + "FROM mytable WHERE mytable.name = :name_1", + ) + + def test_filter_by_from_binary(self): + """test #6414""" + stmt = select(table1.c.myid == 5).filter_by(name="foo") + self.assert_compile( + stmt, + "SELECT mytable.myid = :myid_1 AS anon_1 " + "FROM mytable WHERE mytable.name = :name_1", + ) + + def test_filter_by_from_label(self): + """test #6414""" + stmt = select(table1.c.myid.label("some_id")).filter_by(name="foo") + self.assert_compile( + stmt, + "SELECT mytable.myid AS some_id " + "FROM mytable WHERE mytable.name = :name_1", + ) + + def test_filter_by_no_property_from_table(self): assert_raises_message( exc.InvalidRequestError, 'Entity namespace for "mytable" has no property "foo"', @@ -304,6 +359,14 @@ class FutureSelectTest(fixtures.TestBase, AssertsCompiledSQL): foo="bar", ) + def test_filter_by_no_property_from_col(self): + assert_raises_message( + exc.InvalidRequestError, + 'Entity namespace for "mytable.myid" has no property "foo"', + select(table1.c.myid).filter_by, + foo="bar", + ) + def test_select_tuple_outer(self): stmt = select(tuple_(table1.c.myid, table1.c.name)) |