Fix marker checking when value is Nonenewton-eol4.13.6stable/newton

There are cases where users sort a table using compound-values sort_key
and one of the key has nullable set to True. For example, sorting a
table using ['id', 'updated_at'] where 'updated_at' can be None.

When marker_value is None, we cannot do value comparison using '<' or
'>' operators. This patch adds a check if the value from the marker
corresponding to the nullable-key has None value. If that is the case,
we skip the comparison.

Back to the example above, instead of always getting the following
criteria (which doesn't work):

(id > MARKER_ID) or (id == MARKER_ID && updated_at > None) <-- failure

we will get the following criteria when 'updated_at' is None:

(id > MARKER_ID)

This is not hurting in any way to existing / legal use cases where
callers are expected to include a unique key in sort keys. If there are
such cases, this patch is not making things worse because the sorting
is already unpredictable.

Closes-Bug: #1615938
Change-Id: Iea2cd0bb2556b0b15a0baaa76ef522a3097f9928
(cherry picked from commit b3869d04cff7071c1226758eb8b58fde9eba5b8d)

1 files changed, 20 insertions, 13 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)