Fix searching+sorting for Link properties

Note that this turns off recursive pre-populating the cache in
filter_iter, we only pre-populate the top-level element. Otherwise this
interacts with searching producing duplicates.

Author: schlatterbeck

roundup/backends/rdbms_common.py

@@ -2545,7 +2545,7 @@ def _filter_sql (self, search_matches, filterspec, srt=[], grp=[], retr=0,
         a = self.db.arg
 
         # figure the WHERE clause from the filterspec
-        mlfilt = 0      # are we joining with Multilink tables?
+        use_distinct = False  # Do we need a distinct clause?
         sortattr = self._sortattr (group = grp, sort = srt)
         proptree = self._proptree(filterspec, exact_match_spec, sortattr, retr)
         mlseen = 0
@@ -2581,7 +2581,8 @@ def _filter_sql (self, search_matches, filterspec, srt=[], grp=[], retr=0,
                 rc = oc = ac = '_%s._%s'%(pln, k)
             if isinstance(propclass, Multilink):
                 if 'search' in p.need_for:
-                    mlfilt = 1
+                    # if we joining with Multilink tables we need distinct
+                    use_distinct = True
                     tn = propclass.table_name
                     nid = propclass.nodeid_name
                     lid = propclass.linkid_name
@@ -2672,7 +2673,9 @@ def _filter_sql (self, search_matches, filterspec, srt=[], grp=[], retr=0,
                     if p.children:
                         if 'sort' not in p.need_for:
                             frum.append('_%s as _%s' % (cn, ln))
-                        where.append('_%s._%s=_%s.id'%(pln, k, ln))
+                        c = [x for x in p.children if 'search' in x.need_for]
+                        if c:
+                            where.append('_%s._%s=_%s.id'%(pln, k, ln))
                     if p.has_values:
                         if isinstance(v, type([])):
                             d = {}
@@ -2835,9 +2838,8 @@ def _filter_sql (self, search_matches, filterspec, srt=[], grp=[], retr=0,
             where = ' where ' + (' and '.join(where))
         else:
             where = ''
-        if mlfilt:
-            # we're joining tables on the id, so we will get dupes if we
-            # don't distinct()
+        if use_distinct:
+            # Avoid dupes
             cols[0] = 'distinct(_%s.id)'%icn
 
         order = []

roundup/hyperdb.py

@@ -519,8 +519,9 @@ def append(self, name, need_for='search', retr=False):
         if name in self.propdict:
             pt = self.propdict[name]
             pt.need_for[need_for] = True
-            if retr and isinstance(pt.propclass, Link):
-                pt.append_retr_props()
+            # For now we do not recursively retrieve Link properties
+            #if retr and isinstance(pt.propclass, Link):
+            #    pt.append_retr_props()
             return pt
         propclass = self.props[name]
         cls = None
@@ -538,8 +539,9 @@ def append(self, name, need_for='search', retr=False):
                 child.need_child_retired = True
         self.children.append(child)
         self.propdict[name] = child
-        if retr and isinstance(child.propclass, Link):
-            child.append_retr_props()
+        # For now we do not recursively retrieve Link properties
+        #if retr and isinstance(child.propclass, Link):
+        #    child.append_retr_props()
         return child
 
     def append_retr_props(self):
@@ -1566,8 +1568,7 @@ def _proptree(self, filterspec, exact_match_spec={}, sortattr=[],
         """Build a tree of all transitive properties in the given
         exact_match_spec/filterspec.
         If we retrieve (retr is True) linked items we don't follow
-        across multilinks. We also don't follow if the searched value
-        can contain NULL values.
+        across multilinks or links.
         """
         proptree = Proptree(self.db, self, '', self.getprops(), retr=retr)
         for exact, spec in enumerate((filterspec, exact_match_spec)):

test/db_test_base.py

@@ -3658,14 +3658,14 @@ def testFilteringTransitiveLinkCache(self):
             ('+','username')]):
             result.append(id)
             nodeid = id
-            for x in range(4):
-                assert(('user', nodeid) in self.db.cache)
-                n = self.db.user.getnode(nodeid)
-                for k, v in user_result[nodeid].items():
-                    ae((k, n[k]), (k, v))
-                for k in 'creation', 'activity':
-                    assert(n[k])
-                nodeid = n.supervisor
+            # Note in the recent implementation we do not recursively
+            # cache results in filter_iter
+            assert(('user', nodeid) in self.db.cache)
+            n = self.db.user.getnode(nodeid)
+            for k, v in user_result[nodeid].items():
+                ae((k, n[k]), (k, v))
+            for k in 'creation', 'activity':
+                assert(n[k])
             self.db.clearCache()
         ae (result, ['8', '9', '10', '6', '7', '1', '3', '2', '4', '5'])
 
@@ -3683,14 +3683,13 @@ def testFilteringTransitiveLinkCache(self):
             for k in 'creation', 'activity':
                 assert(n[k])
             nodeid = n.assignedto
-            for x in range(4):
-                assert(('user', nodeid) in self.db.cache)
-                n = self.db.user.getnode(nodeid)
-                for k, v in user_result[nodeid].items():
-                    ae((k, n[k]), (k, v))
-                for k in 'creation', 'activity':
-                    assert(n[k])
-                nodeid = n.supervisor
+            # Note in the recent implementation we do not recursively
+            # cache results in filter_iter
+            n = self.db.user.getnode(nodeid)
+            for k, v in user_result[nodeid].items():
+                ae((k, n[k]), (k, v))
+            for k in 'creation', 'activity':
+                assert(n[k])
             self.db.clearCache()
         ae (result, ['4', '5', '6', '7', '8', '1', '2', '3'])