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 (file)
index 0000000..317816e
--- /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 721a4794c5fd377f6b33ae59badf3dfee81430a2..530b8fee880fad50c70e730ab7a26b58479ae7bf 100644 (file)
--- 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 3a216174065086fddfdd108f0a63478562dfdd15..cdc7e4af42b9d2e0c0cae7f342a66ab0f2eecca6 100644 (file)
--- 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 879af1197bfe9f0d9fb75487569cad83ea447a41..9502b16b73624d5419f00cd52c1a8e600653c5f2 100644 (file)
--- 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 54b2afb6f9fd9f4073e2c92867b82971f6f161b2..35328e98495424f506c3b2c1893229cb7fcdc995 100644 (file)
--- 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(