Null-value sorting fixes:

- Fixed sorting of NULL values for the PostgreSQL backend. Generic
  implementation of additional sort-key for NULL values, can be reused
  by other (future) rdbms backends.
- Added a test for NULL value sorting and removed an override for the
  Interval test for the PostgreSQL backend which had a special case for
  NULL values

Author: Ralf Schlatterbeck

roundup/backends/back_postgresql.py

@@ -1,4 +1,4 @@
-#$Id: back_postgresql.py,v 1.32 2006-08-11 00:44:00 richard Exp $
+#$Id: back_postgresql.py,v 1.33 2006-08-23 12:57:10 schlatterbeck Exp $
 #
 # Copyright (c) 2003 Martynas Sklyzmantas, Andrey Lebedev <[email protected]>
 #
@@ -223,11 +223,13 @@ def clear(self):
             self.cursor.execute('DROP SEQUENCE _%s_ids'%cn)
             self.cursor.execute('CREATE SEQUENCE _%s_ids'%cn)
 
+class PostgresqlClass:
+    order_by_null_values = '(%s is not NULL)'
 
-class Class(rdbms_common.Class):
+class Class(PostgresqlClass, rdbms_common.Class):
     pass
-class IssueClass(rdbms_common.IssueClass):
+class IssueClass(PostgresqlClass, rdbms_common.IssueClass):
     pass
-class FileClass(rdbms_common.FileClass):
+class FileClass(PostgresqlClass, rdbms_common.FileClass):
     pass
 

roundup/backends/rdbms_common.py

@@ -15,7 +15,7 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 #
-#$Id: rdbms_common.py,v 1.176 2006-08-22 19:27:36 schlatterbeck Exp $
+#$Id: rdbms_common.py,v 1.177 2006-08-23 12:57:10 schlatterbeck Exp $
 ''' Relational database (SQL) backend common code.
 
 Basics:
@@ -2024,6 +2024,14 @@ def _subselect(self, classname, multilink_table):
         return '_%s.id not in (select nodeid from %s)'%(classname,
             multilink_table)
 
+    # Some DBs order NULL values last. Set this variable in the backend
+    # for prepending an order by clause for each attribute that causes
+    # correct sort order for NULLs. Examples:
+    # order_by_null_values = '(%s is not NULL)'
+    # order_by_null_values = 'notnull(%s)'
+    # The format parameter is replaced with the attribute.
+    order_by_null_values = None
+
     def filter(self, search_matches, filterspec, sort=[], group=[]):
         '''Return a list of the ids of the active nodes in this class that
         match the 'filter' spec, sorted by the group spec and then the
@@ -2242,9 +2250,13 @@ def filter(self, search_matches, filterspec, sort=[], group=[]):
                     # Don't select top-level id twice
                     if p.name != 'id' or p.parent != proptree:
                         ordercols.append(oc)
-                    if p.sort_direction == '-':
-                        oc += ' desc'
-                    orderby.append(oc)
+                    desc = ['', ' desc'][p.sort_direction == '-']
+                    # Some SQL dbs sort NULL values last -- we want them first.
+                    if (self.order_by_null_values and p.name != 'id'):
+                        nv = self.order_by_null_values % oc
+                        ordercols.append(nv)
+                        orderby.append(nv + desc)
+                    orderby.append(oc + desc)
 
         props = self.getprops()
 

test/db_test_base.py

@@ -15,7 +15,7 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 #
-# $Id: db_test_base.py,v 1.76 2006-08-22 19:33:02 schlatterbeck Exp $
+# $Id: db_test_base.py,v 1.77 2006-08-23 12:57:10 schlatterbeck Exp $
 
 import unittest, os, shutil, errno, imp, sys, time, pprint, sets
 
@@ -1216,8 +1216,8 @@ def testFilteringTransitiveLinkSort(self):
         ae, filt = self.filteringSetupTransitiveSearch()
         ufilt = self.db.user.filter
         # Need to make ceo his own (and first two users') supervisor,
-        # otherwise sorting for postgreSQL will fail: postgreSQL seems
-        # to sort NULLs last.
+        # otherwise we will depend on sorting order of NULL values.
+        # Leave that to a separate test.
         self.db.user.set('1', supervisor = '3')
         self.db.user.set('2', supervisor = '3')
         self.db.user.set('3', supervisor = '3')
@@ -1253,6 +1253,27 @@ def testFilteringTransitiveLinkSort(self):
             ('-','assignedto.supervisor'), ('+','assignedto'), ('+','status')]),
             ['4', '5', '7', '6', '8', '1', '2', '3'])
 
+    def testFilteringTransitiveLinkSortNull(self):
+        """Check sorting of NULL values"""
+        ae, filt = self.filteringSetupTransitiveSearch()
+        ufilt = self.db.user.filter
+        ae(ufilt(None, {}, [('+','supervisor.supervisor.supervisor'),
+            ('+','supervisor.supervisor'), ('+','supervisor'),
+            ('+','username')]),
+            ['1', '3', '2', '4', '5', '6', '7', '8', '9', '10'])
+        ae(ufilt(None, {}, [('+','supervisor.supervisor.supervisor'),
+            ('-','supervisor.supervisor'), ('-','supervisor'),
+            ('+','username')]),
+            ['8', '9', '10', '6', '7', '4', '5', '1', '3', '2'])
+        ae(filt(None, {}, [('+','assignedto.supervisor.supervisor.supervisor'),
+            ('+','assignedto.supervisor.supervisor'),
+            ('+','assignedto.supervisor'), ('+','assignedto')]),
+            ['1', '2', '3', '4', '5', '6', '7', '8'])
+        ae(filt(None, {}, [('+','assignedto.supervisor.supervisor.supervisor'),
+            ('+','assignedto.supervisor.supervisor'),
+            ('-','assignedto.supervisor'), ('+','assignedto')]),
+            ['4', '5', '6', '7', '8', '1', '2', '3'])
+
     def testFilteringTransitiveLinkIssue(self):
         ae, filt = self.filteringSetupTransitiveSearch()
         ae(filt(None, {'assignedto.supervisor.username': 'grouplead1'},

test/test_postgresql.py

@@ -15,7 +15,7 @@
 # BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
 #
-# $Id: test_postgresql.py,v 1.12 2004-11-03 01:34:21 richard Exp $
+# $Id: test_postgresql.py,v 1.13 2006-08-23 12:57:10 schlatterbeck Exp $
 
 import unittest
 
@@ -48,15 +48,6 @@ def tearDown(self):
         DBTest.tearDown(self)
         postgresqlOpener.tearDown(self)
 
-    def testFilteringIntervalSort(self):
-        # PostgreSQL sorts NULLs differently to other databases (others
-        # treat it as lower than real values, PG treats it as higher)
-        ae, filt = self.filteringSetup()
-        # ascending should sort None, 1:10, 1d
-        ae(filt(None, {}, ('+','foo'), (None,None)), ['4', '1', '2', '3'])
-        # descending should sort 1d, 1:10, None
-        ae(filt(None, {}, ('-','foo'), (None,None)), ['3', '2', '1', '4'])
-
 class postgresqlROTest(postgresqlOpener, ROTest):
     def setUp(self):
         postgresqlOpener.setUp(self)