Merge ~cjwatson/launchpad:optimize-person-sorting into launchpad:master

[Colin Watson]

Optimize short-circuit case in PersonSet.findPerson

Ordering this query by `Person._storm_sortingColumns` (a somewhat dubiously-named attribute that is in fact just `(Person.displayname, Person.name)` without the `person_sort_key` function) causes PostgreSQL to use a very slow plan that has to sort the whole `Person` table before applying the row limit:

   Limit (cost=798114.96..798115.15 rows=76 width=1049)
     -> Sort (cost=798114.96..806603.87 rows=3395564 width=1049)
           Sort Key: person.displayname, person.name
           -> Merge Join (cost=1.66..675060.95 rows=3395564 width=1049)
                 Merge Cond: (account.id = person.account)
                 -> Index Scan using account_pkey on account (cost=0.43..287109.00 rows=3416242 width=4)
                       Filter: (status <> ALL ('{5,30,40,50,60}'::integer[]))
                 -> Index Scan using person__account__key on person (cost=0.43..330359.42 rows=7441191 width=1049)
                       Filter: ((teamowner IS NULL) AND (merged IS NULL))

Using `Person.sortingColumns` instead is more in line with what this method does in the non-short-circuit case, and it produces a much more sensible plan:

   Limit (cost=0.99..111.69 rows=76 width=1081)
     -> Nested Loop (cost=0.99..4946116.34 rows=3395564 width=1081)
           -> Index Scan using person_sorting_idx on person (cost=0.56..505451.00 rows=7441191 width=1049)
                 Filter: ((teamowner IS NULL) AND (merged IS NULL))
           -> Index Scan using account_pkey on account (cost=0.43..0.48 rows=1 width=4)
                 Index Cond: (id = person.account)
                 Filter: (status <> ALL ('{5,30,40,50,60}'::integer[]))

[Colin Watson (cjwatson)]

This is pretty simple (at least after digging through the SQL details), and there's been plenty of time for objections, so I'm going to go ahead with this.

review: Approve

diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 175ccfc..1eef067 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -4417,7 +4417,7 @@ class PersonSet:
         # Short circuit for returning all users in order
         if not text:
             results = store.find(Person, base_query)
-            return results.order_by(Person._storm_sortingColumns)
+            return results.order_by(Person.sortingColumns)
 
         # We use a UNION here because this makes things *a lot* faster
         # than if we did a single SELECT with the two following clauses