From adeaa7bdcc6f5af18b0a8016864f5cbbc08d92f0 Mon Sep 17 00:00:00 2001
From: Kevin Benton <kevin@benton.pub>
Date: Mon, 16 Jan 2017 14:09:36 -0800
Subject: [PATCH] Coerce booleans to integer values in paginate_query

This will avoid a traceback resulting from using > or <
on a boolean value (example in bug report).

Change-Id: I4b90f18737e62bb35bacf75ad7a85603fb7b45cc
Closes-Bug: #1656947
---
 oslo_db/sqlalchemy/utils.py            | 11 ++++++++---
 oslo_db/tests/sqlalchemy/test_utils.py | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py
index 5adbd224..97a28c61 100644
--- a/oslo_db/sqlalchemy/utils.py
+++ b/oslo_db/sqlalchemy/utils.py
@@ -38,6 +38,7 @@ from sqlalchemy import Index
 from sqlalchemy import inspect
 from sqlalchemy import Integer
 from sqlalchemy import MetaData
+from sqlalchemy.sql.expression import cast
 from sqlalchemy.sql.expression import literal_column
 from sqlalchemy.sql import text
 from sqlalchemy import String
@@ -236,11 +237,15 @@ def paginate_query(query, model, limit, sort_keys, marker=None,
                     crit_attrs.append((model_attr == marker_values[j]))
 
                 model_attr = getattr(model, sort_keys[i])
+                val = marker_values[i]
+                # sqlalchemy doesn't like booleans in < >. bug/1656947
+                if isinstance(model_attr.type, Boolean):
+                    val = int(val)
+                    model_attr = cast(model_attr, Integer)
                 if sort_dirs[i].startswith('desc'):
-                    crit_attrs.append((model_attr < marker_values[i]))
+                    crit_attrs.append((model_attr < val))
                 else:
-                    crit_attrs.append((model_attr > marker_values[i]))
-
+                    crit_attrs.append((model_attr > val))
                 criteria = sqlalchemy.sql.and_(*crit_attrs)
                 criteria_list.append(criteria)
 
diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py
index c88ac106..40a7ea25 100644
--- a/oslo_db/tests/sqlalchemy/test_utils.py
+++ b/oslo_db/tests/sqlalchemy/test_utils.py
@@ -32,6 +32,7 @@ from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import mapper
 from sqlalchemy.orm import Session
+from sqlalchemy.sql.expression import cast
 from sqlalchemy.sql import select
 from sqlalchemy.types import UserDefinedType, NullType
 from sqlalchemy.dialects.postgresql import psycopg2
@@ -77,6 +78,7 @@ class FakeTable(Base):
     project_id = Column(String(50))
     snapshot_id = Column(String(50))
     updated_at = Column(DateTime, nullable=True)
+    enabled = Column(Boolean, default=True)
 
     # mox is comparing in some awkward way that
     # in this case requires the same identity of object
@@ -450,6 +452,25 @@ class TestGetUniqueKeys(test_base.BaseTestCase):
 
 class TestPaginateQueryActualSQL(test_base.BaseTestCase):
 
+    def test_paginate_with_boolean_sort(self):
+        s = Session()
+        q = s.query(FakeTable)
+        q = utils.paginate_query(q, FakeTable, 5, ['enabled'],
+                                 sort_dirs=['asc'],
+                                 marker=FakeTable(user_id='hello',
+                                                  enabled=False))
+        expected_core_sql = (
+            select([FakeTable]).
+            order_by(sqlalchemy.asc(FakeTable.enabled)).
+            where(cast(FakeTable.enabled, Integer) > 0).
+            limit(5)
+        )
+
+        self.assertEqual(
+            str(expected_core_sql.compile()),
+            str(q.statement.compile())
+        )
+
     def test_paginate_on_hybrid_assert_stmt(self):
         s = Session()
         q = s.query(FakeTable)