From 2cf518554d23527e63ea64cf1e73fd7aa0aed6ba Mon Sep 17 00:00:00 2001
From: Kevin Benton <kevin@benton.pub>
Date: Mon, 16 Jan 2017 14:16:49 -0800
Subject: [PATCH] Protect against non-determinstic sort

This reverts commit e74b45d0eb923129902f1926aaddb51e6e2b8085.

This re-adds the non-deterministic sort logic but avoids using
boolean columns to work around the related bug.

Original commit msg:

With the switch to subquery loading, we need to eliminate the
possibility for non-deterministic results in the event of LIMIT
queries caused by non-unique SORT BY parameters.

This always adds on a set of unique keys to sort by when sorting
params are present. By placing them at the end, they take the lower
priority over the user-provided sort strings.

1. http://docs.sqlalchemy.org/en/latest/orm/loading_relationships.html#subqueryload-ordering
2. http://docs.sqlalchemy.org/en/latest/faq/ormconfiguration.html#faq-subqueryload-limit-sort

Related-Bug: #1656947
Change-Id: I503c65560d970427be93b39898f4e7acfc4d3e1f
(cherry picked from commit 393a3bd94724284c8f567fbfcd418859e5fc73c4)
---
 neutron/db/common_db_mixin.py | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/neutron/db/common_db_mixin.py b/neutron/db/common_db_mixin.py
index 5efdc09fd2a..d7177bee549 100644
--- a/neutron/db/common_db_mixin.py
+++ b/neutron/db/common_db_mixin.py
@@ -287,12 +287,39 @@ class CommonDbMixin(object):
         if sorts:
             sort_keys = db_utils.get_and_validate_sort_keys(sorts, model)
             sort_dirs = db_utils.get_sort_dirs(sorts, page_reverse)
+            if limit:
+                # we always want deterministic results for limit subqueries
+                # so add unique keys to limit queries when present.
+                # (http://docs.sqlalchemy.org/en/latest/orm/
+                #  loading_relationships.html#subqueryload-ordering)
+                # (http://docs.sqlalchemy.org/en/latest/faq/
+                #  ormconfiguration.html#faq-subqueryload-limit-sort)
+                for k in self._unique_keys(model, marker_obj):
+                    if k not in sort_keys:
+                        sort_keys.append(k)
+                        sort_dirs.append('asc')
             collection = sa_utils.paginate_query(collection, model, limit,
                                                  marker=marker_obj,
                                                  sort_keys=sort_keys,
                                                  sort_dirs=sort_dirs)
         return collection
 
+    def _unique_keys(self, model, marker_obj):
+        # just grab first set of unique keys and use them.
+        # if model has no unqiue sets, 'paginate_query' will
+        # warn if sorting is unstable
+        uk_sets = sa_utils._get_unique_keys(model)
+        for kset in uk_sets:
+            for k in kset:
+                if marker_obj and isinstance(getattr(marker_obj, k), bool):
+                    # TODO(kevinbenton): workaround for bug/1656947.
+                    # we can't use boolean cols until that bug is fixed. return
+                    # first entry in uk_sets once that bug is resolved
+                    break
+            else:
+                return kset
+        return []
+
     def _get_collection(self, context, model, dict_func, filters=None,
                         fields=None, sorts=None, limit=None, marker_obj=None,
                         page_reverse=False):