diff options
| -rw-r--r-- | oslo_db/sqlalchemy/utils.py | 33 | ||||
| -rw-r--r-- | oslo_db/tests/sqlalchemy/test_utils.py | 33 |
2 files changed, 52 insertions, 14 deletions
diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index c5030a0..e628908 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -146,7 +146,8 @@ def paginate_query(query, model, limit, sort_keys, marker=None, the lexicographical ordering: (k1 > X1) or (k1 == X1 && k2 > X2) or (k1 == X1 && k2 == X2 && k3 > X3) - We also have to cope with different sort_directions. + We also have to cope with different sort_directions and cases where k2, + k3, ... are nullable. Typically, the id of the last row is used as the client-facing pagination marker, then the actual marker object must be fetched from the db and @@ -223,18 +224,24 @@ def paginate_query(query, model, limit, sort_keys, marker=None, criteria_list = [] for i in range(len(sort_keys)): crit_attrs = [] - for j in range(i): - model_attr = getattr(model, sort_keys[j]) - crit_attrs.append((model_attr == marker_values[j])) - - model_attr = getattr(model, sort_keys[i]) - if sort_dirs[i].startswith('desc'): - crit_attrs.append((model_attr < marker_values[i])) - else: - crit_attrs.append((model_attr > marker_values[i])) - - criteria = sqlalchemy.sql.and_(*crit_attrs) - criteria_list.append(criteria) + # NOTE: We skip the marker value comparison if marker_values[i] is + # None, for two reasons: 1) the comparison operators below + # ('<', '>') are not applicable on None value; 2) this is + # safe because we can assume the primary key is included in + # sort_key, thus checked as (one of) marker values. + if marker_values[i] is not None: + for j in range(i): + model_attr = getattr(model, sort_keys[j]) + crit_attrs.append((model_attr == marker_values[j])) + + model_attr = getattr(model, sort_keys[i]) + if sort_dirs[i].startswith('desc'): + crit_attrs.append((model_attr < marker_values[i])) + else: + crit_attrs.append((model_attr > marker_values[i])) + + criteria = sqlalchemy.sql.and_(*crit_attrs) + criteria_list.append(criteria) f = sqlalchemy.sql.or_(*criteria_list) query = query.filter(f) diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 31686fe..fc7eb03 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -77,6 +77,7 @@ class FakeTable(Base): user_id = Column(String(50), primary_key=True) project_id = Column(String(50)) snapshot_id = Column(String(50)) + updated_at = Column(DateTime, nullable=True) # mox is comparing in some awkward way that # in this case requires the same identity of object @@ -171,7 +172,8 @@ class TestPaginateQuery(test_base.BaseTestCase): self.mox.StubOutWithMock(sqlalchemy, 'desc') self.marker = FakeTable(user_id='user', project_id='p', - snapshot_id='s') + snapshot_id='s', + updated_at=None) self.model = FakeTable def test_paginate_query_no_pagination_no_sort_dirs(self): @@ -301,6 +303,35 @@ class TestPaginateQuery(test_base.BaseTestCase): marker=self.marker, sort_dirs=['asc-nullslast', 'desc-nullsfirst']) + def test_paginate_query_marker_null(self): + self.mox.StubOutWithMock(self.model.user_id, 'isnot') + self.model.user_id.isnot(None).AndReturn('asc_null_1') + sqlalchemy.desc('asc_null_1').AndReturn('asc_null_2') + self.query.order_by('asc_null_2').AndReturn(self.query) + + sqlalchemy.asc(self.model.user_id).AndReturn('asc_1') + self.query.order_by('asc_1').AndReturn(self.query) + + self.mox.StubOutWithMock(self.model.updated_at, 'is_') + self.model.updated_at.is_(None).AndReturn('desc_null_1') + sqlalchemy.desc('desc_null_1').AndReturn('desc_null_2') + self.query.order_by('desc_null_2').AndReturn(self.query) + + sqlalchemy.desc(self.model.updated_at).AndReturn('desc_1') + self.query.order_by('desc_1').AndReturn(self.query) + + self.mox.StubOutWithMock(sqlalchemy.sql, 'and_') + sqlalchemy.sql.and_(mock.ANY).AndReturn('some_crit') + self.mox.StubOutWithMock(sqlalchemy.sql, 'or_') + sqlalchemy.sql.or_('some_crit').AndReturn('some_f') + self.query.filter('some_f').AndReturn(self.query) + self.query.limit(5).AndReturn(self.query) + self.mox.ReplayAll() + utils.paginate_query(self.query, self.model, 5, + ['user_id', 'updated_at'], + marker=self.marker, + sort_dirs=['asc-nullslast', 'desc-nullsfirst']) + def test_paginate_query_value_error(self): sqlalchemy.asc(self.model.user_id).AndReturn('asc_1') self.query.order_by('asc_1').AndReturn(self.query) |