Rewrite main CF dashboard query to use SQL

We've long gone past what the django ORM can handle and got into
hundreds of queries for large commitfests. And it's not really that hard
to rewrite as SQL, even though we have to dynamically build the WHERE
clauses. So do that.

diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py
index 5936b65c7610ead10950b2d4232d7290fd8aa0dc..8e5a442926ee1646759d46687e0304d1d5285fc1 100644 (file)
--- a/pgcommitfest/commitfest/models.py
+++ b/pgcommitfest/commitfest/models.py
@@ -177,7 +177,7 @@ class PatchOnCommitFest(models.Model):
                (STATUS_REJECTED, 'danger'),
                (STATUS_RETURNED, 'danger'),
        )
-       OPEN_STATUSES=(STATUS_REVIEW, STATUS_AUTHOR, STATUS_COMMITTER)
+       OPEN_STATUSES=[STATUS_REVIEW, STATUS_AUTHOR, STATUS_COMMITTER]
        OPEN_STATUS_CHOICES=[x for x in _STATUS_CHOICES if x[0] in OPEN_STATUSES]
 
        patch = models.ForeignKey(Patch, blank=False, null=False)

diff --git a/pgcommitfest/commitfest/templates/commitfest.html b/pgcommitfest/commitfest/templates/commitfest.html
index 009ff5e35df835e0387c2f45c58fde3d81773578..5a7f787f856e44af63137cd1a37a92e2a4ece46a 100644 (file)
--- a/pgcommitfest/commitfest/templates/commitfest.html
+++ b/pgcommitfest/commitfest/templates/commitfest.html
@@ -82,7 +82,7 @@
 {%endifchanged%} 
 {%endif%}
   <tr>
-   <td><a href="{{p.id}}/">{{p}}</a></td>
+   <td><a href="{{p.id}}/">{{p.name}}</a></td>
    <td><span class="label label-{{p.status|patchstatuslabel}}">{{p.status|patchstatusstring}}</span></td>
    <td>{{p.author_names|default:''}}</td>
    <td>{{p.reviewer_names|default:''}}</td>

diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py
index 5a5505f89067f1d5629d5fa19d44767e5c93ab0c..4f3183e6c894e031f935e5c6eadd3cc14516889b 100644 (file)
--- a/pgcommitfest/commitfest/views.py
+++ b/pgcommitfest/commitfest/views.py
@@ -104,48 +104,55 @@ def commitfest(request, cfid):
        cf = get_object_or_404(CommitFest, pk=cfid)
 
        # Build a dynamic filter based on the filtering options entered
-       q = Q()
+       whereclauses = []
+       whereparams = {}
        if request.GET.has_key('status') and request.GET['status'] != "-1":
                try:
-                       q = q & Q(patchoncommitfest__status=int(request.GET['status']))
+                       whereclauses.append("poc.status=%(status)s")
+                       whereparams['status'] = int(request.GET['status'])
                except ValueError:
                        # int() failed -- so just ignore this filter
                        pass
 
        if request.GET.has_key('author') and request.GET['author'] != "-1":
                if request.GET['author'] == '-2':
-                       q = q & Q(authors=None)
+                       whereclauses.append("NOT EXISTS (SELECT 1 FROM commitfest_patch_authors cpa WHERE cpa.patch_id=p.id)")
                elif request.GET['author'] == '-3':
                        # Checking for "yourself" requires the user to be logged in!
                        if not request.user.is_authenticated():
                                return HttpResponseRedirect('%s?next=%s' % (settings.LOGIN_URL, request.path))
-                       q = q & Q(authors=request.user)
+                       whereclauses.append("EXISTS (SELECT 1 FROM commitfest_patch_authors cpa WHERE cpa.patch_id=p.id AND cpa.user_id=%(self)s)")
+                       whereparams['self'] = request.user.id
                else:
                        try:
-                               q = q & Q(authors__id=int(request.GET['author']))
+                               whereclauses.append("EXISTS (SELECT 1 FROM commitfest_patch_authors cpa WHERE cpa.patch_id=p.id AND cpa.user_id=%(author)s)")
+                               whereparams['author'] = int(request.GET['author'])
                        except ValueError:
                                # int() failed -- so just ignore this filter
                                pass
 
        if request.GET.has_key('reviewer') and request.GET['reviewer'] != "-1":
                if request.GET['reviewer'] == '-2':
-                       q = q & Q(reviewers=None)
+                       whereclauses.append("NOT EXISTS (SELECT 1 FROM commitfest_patch_reviewers cpr WHERE cpr.patch_id=p.id)")
                elif request.GET['reviewer'] == '-3':
                        # Checking for "yourself" requires the user to be logged in!
                        if not request.user.is_authenticated():
                                return HttpResponseRedirect('%s?next=%s' % (settings.LOGIN_URL, request.path))
-                       q = q & Q(reviewers=request.user)
+                       whereclauses.append("EXISTS (SELECT 1 FROM commitfest_patch_reviewers cpr WHERE cpr.patch_id=p.id AND cpr.user_id=%(self)s)")
+                       whereparams['self'] = request.user.id
                else:
                        try:
-                               q = q & Q(reviewers__id=int(request.GET['reviewer']))
+                               whereclauses.append("EXISTS (SELECT 1 FROM commitfest_patch_reviewers cpr WHERE cpr.patch_id=p.id AND cpr.user_id=%(reviewer)s)")
+                               whereparams['reviewer'] = int(request.GET['reviewer'])
                        except ValueError:
                                # int() failed -- so just ignore this filter
                                pass
 
        if request.GET.has_key('text') and request.GET['text'] != '':
-               q = q & Q(name__icontains=request.GET['text'])
+               whereclauses.append("p.name ILIKE '%%' || %(txt)s || '%%'")
+               whereparams['txt'] = request.GET['text']
 
-       has_filter = len(q.children) > 0
+       has_filter = len(whereclauses) > 0
 
        # Figure out custom ordering
        ordering = ['-is_open', 'topic__topic', 'created',]
@@ -156,27 +163,46 @@ def commitfest(request, cfid):
                        sortkey=0
 
                if sortkey==1:
-                       ordering = ['-is_open', 'modified', 'created',]
+                       orderby_str = 'modified, created'
                elif sortkey==2:
-                       ordering = ['-is_open', 'lastmail', 'created',]
+                       orderby_str = 'lastmail, created'
                elif sortkey==3:
-                       ordering = ['-is_open', '-num_cfs', 'modified', 'created']
+                       orderby_str = 'num_cfs DESC, modified, created'
                else:
+                       orderby_str = 'p.id'
                        sortkey=0
        else:
+               orderby_str = 'p.id'
                sortkey = 0
 
        if not has_filter and sortkey==0 and request.GET:
                # Redirect to get rid of the ugly url
                return HttpResponseRedirect('/%s/' % cf.id)
 
-       patches = list(cf.patch_set.filter(q).select_related().extra(select={
-               'status':'commitfest_patchoncommitfest.status',
-               'author_names':"SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_authors cpa ON cpa.user_id=auth_user.id WHERE cpa.patch_id=commitfest_patch.id",
-               'reviewer_names':"SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_reviewers cpr ON cpr.user_id=auth_user.id WHERE cpr.patch_id=commitfest_patch.id",
-               'is_open':'commitfest_patchoncommitfest.status IN (%s)' % ','.join([str(x) for x in PatchOnCommitFest.OPEN_STATUSES]),
-               'num_cfs': 'SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=commitfest_patch.id',
-       }).order_by(*ordering))
+       if whereclauses:
+               where_str = 'AND ({0})'.format(' AND '.join(whereclauses))
+       else:
+               where_str = ''
+       params = {
+               'cid': cf.id,
+               'openstatuses': PatchOnCommitFest.OPEN_STATUSES,
+       }
+       params.update(whereparams)
+
+       # Let's not overload the poor django ORM
+       curs = connection.cursor()
+       curs.execute("""SELECT p.id, p.name, poc.status, p.created, p.modified, p.lastmail, committer.username AS committer,
+(poc.status=ANY(%(openstatuses)s)) AS is_open,
+(SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_authors cpa ON cpa.user_id=auth_user.id WHERE cpa.patch_id=p.id) AS author_names,
+(SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_reviewers cpr ON cpr.user_id=auth_user.id WHERE cpr.patch_id=p.id) AS reviewer_names,
+(SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=p.id) AS num_cfs
+FROM commitfest_patch p
+INNER JOIN commitfest_patchoncommitfest poc ON poc.patch_id=p.id
+LEFT JOIN auth_user committer ON committer.id=p.committer_id
+WHERE poc.commitfest_id=%(cid)s {0}
+GROUP BY p.id, poc.id, committer.id
+ORDER BY is_open DESC, {1}""".format(where_str, orderby_str), params)
+       patches = [dict(zip([col[0] for col in curs.description], row)) for row in curs.fetchall()]
 
        # Generate patch status summary.
        curs = connection.cursor()
@@ -199,7 +225,7 @@ def commitfest(request, cfid):
                'title': cf.title,
                'grouping': sortkey==0,
                'sortkey': sortkey,
-               'openpatchids': [p.id for p in patches if p.is_open],
+               'openpatchids': [p['id'] for p in patches if p['is_open']],
                'header_activity': 'Activity log',
                'header_activity_link': 'activity/',
                })