From: Mike Bayer Date: Sun, 11 Aug 2019 14:54:49 +0000 (-0400) Subject: Use ternary when running conditional with Query._offset X-Git-Tag: rel_1_3_7~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c37c31a90fad5b6da5b73882afddb0e5d292d2f8;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Use ternary when running conditional with Query._offset Fixed bug where using :meth:`.Query.first` or a slice expression in conjunction with a query that has an expression based "offset" applied would raise TypeError, due to an "or" conditional against "offset" that did not expect it to be a SQL expression as opposed to an integer or None. Fixes: #4803 Change-Id: I56b97a5d23cb45427a27a90ab557fa1ac5c6739e (cherry picked from commit 97d2a2091ed4caee1e19168d0db39e4d94a6d12f) --- diff --git a/doc/build/changelog/unreleased_13/4803.rst b/doc/build/changelog/unreleased_13/4803.rst new file mode 100644 index 0000000000..317816ea87 --- /dev/null +++ b/doc/build/changelog/unreleased_13/4803.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm + :tickets: 4803 + + Fixed bug where using :meth:`.Query.first` or a slice expression in + conjunction with a query that has an expression based "offset" applied + would raise TypeError, due to an "or" conditional against "offset" that did + not expect it to be a SQL expression as opposed to an integer or None. + diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 721a4794c5..530b8fee88 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -3054,14 +3054,18 @@ class Query(object): """ if start is not None and stop is not None: - self._offset = (self._offset or 0) + start + self._offset = self._offset if self._offset is not None else 0 + if start != 0: + self._offset += start self._limit = stop - start elif start is None and stop is not None: self._limit = stop elif start is not None and stop is None: - self._offset = (self._offset or 0) + start + self._offset = self._offset if self._offset is not None else 0 + if start != 0: + self._offset += start - if self._offset == 0: + if isinstance(self._offset, int) and self._offset == 0: self._offset = None @_generative(_no_statement_condition) diff --git a/lib/sqlalchemy/testing/requirements.py b/lib/sqlalchemy/testing/requirements.py index 3a21617406..cdc7e4af42 100644 --- a/lib/sqlalchemy/testing/requirements.py +++ b/lib/sqlalchemy/testing/requirements.py @@ -111,6 +111,15 @@ class SuiteRequirements(Requirements): return exclusions.open() + @property + def sql_expression_limit_offset(self): + """target database can render LIMIT and/or OFFSET with a complete + SQL expression, such as one that uses the addition operator. + parameter + """ + + return exclusions.open() + @property def parens_in_union_contained_select_w_limit_offset(self): """Target database must support parenthesized SELECT in UNION diff --git a/test/orm/test_query.py b/test/orm/test_query.py index 879af1197b..9502b16b73 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -2394,6 +2394,8 @@ class ComparatorTest(QueryTest): # more slice tests are available in test/orm/generative.py class SliceTest(QueryTest): + __dialect__ = "default" + def test_first(self): User = self.classes.User @@ -2496,6 +2498,84 @@ class SliceTest(QueryTest): ], ) + def test_first_against_expression_offset(self): + User = self.classes.User + + sess = create_session() + q = sess.query(User).order_by(User.id).offset(literal_column("2")) + + self.assert_sql( + testing.db, + q.first, + [ + ( + "SELECT users.id AS users_id, users.name AS users_name " + "FROM users ORDER BY users.id " + "LIMIT :param_1 OFFSET 2", + [{"param_1": 1}], + ) + ], + ) + + @testing.requires.sql_expression_limit_offset + def test_full_slice_against_expression_offset(self): + User = self.classes.User + + sess = create_session() + q = sess.query(User).order_by(User.id).offset(literal_column("2")) + + self.assert_sql( + testing.db, + lambda: q[2:5], + [ + ( + "SELECT users.id AS users_id, users.name AS users_name " + "FROM users ORDER BY users.id " + "LIMIT :param_1 OFFSET 2 + :2_1", + [{"param_1": 3, "2_1": 2}], + ) + ], + ) + + def test_full_slice_against_integer_offset(self): + User = self.classes.User + + sess = create_session() + q = sess.query(User).order_by(User.id).offset(2) + + self.assert_sql( + testing.db, + lambda: q[2:5], + [ + ( + "SELECT users.id AS users_id, users.name AS users_name " + "FROM users ORDER BY users.id " + "LIMIT :param_1 OFFSET :param_2", + [{"param_1": 3, "param_2": 4}], + ) + ], + ) + + @testing.requires.sql_expression_limit_offset + def test_start_slice_against_expression_offset(self): + User = self.classes.User + + sess = create_session() + q = sess.query(User).order_by(User.id).offset(literal_column("2")) + + self.assert_sql( + testing.db, + lambda: q[2:], + [ + ( + "SELECT users.id AS users_id, users.name AS users_name " + "FROM users ORDER BY users.id " + "LIMIT -1 OFFSET 2 + :2_1", + [{"2_1": 2}], + ) + ], + ) + class FilterTest(QueryTest, AssertsCompiledSQL): __dialect__ = "default" diff --git a/test/requirements.py b/test/requirements.py index 54b2afb6f9..35328e9849 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -605,6 +605,16 @@ class DefaultRequirements(SuiteRequirements): equivalent to a result set.""" return fails_if(["sybase"], "no support for OFFSET or equivalent") + @property + def sql_expression_limit_offset(self): + return ( + fails_if( + ["mysql"], + "MySQL can't accommodate full expressions in OFFSET or LIMIT", + ) + + self.offset + ) + @property def window_functions(self): return only_if(