Merge lp:~camptocamp/openobject-addons/7.0_fix-document-search-order-by_rde into lp:openobject-addons/7.0

Proposed by Alexandre Fayolle - camptocamp
Status: Needs review
Proposed branch: lp:~camptocamp/openobject-addons/7.0_fix-document-search-order-by_rde
Merge into: lp:openobject-addons/7.0
Diff against target: 25 lines (+7/-1)
1 file modified
document/document.py (+7/-1)
To merge this branch: bzr merge lp:~camptocamp/openobject-addons/7.0_fix-document-search-order-by_rde
Reviewer Review Type Date Requested Status
Yannick Vaucher @ Camptocamp (community) code review, no test Approve
Leonardo Pistone (community) code review Approve
OpenERP Core Team Pending
Review via email: mp+214483@code.launchpad.net

Description of the change

[FIX] document: preserve the order in search()

To post a comment you must log in.
Revision history for this message
Leonardo Pistone (lepistone) wrote :

Thanks for pointing out the performance problems of list.remove, good catch.

I suppose the list comprehension at the end is necessary because there is no "intersection" method between sets and lists that preserves order.

review: Approve (code review)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

LGTM

Thanks

review: Approve (code review, no test)

Unmerged revisions

9957. By Romain Deheele - Camptocamp on 2014-04-04

[FIX] document: search method overload should take into account order by clause

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'document/document.py'
2--- document/document.py 2013-12-16 12:39:54 +0000
3+++ document/document.py 2014-04-07 08:30:58 +0000
4@@ -89,6 +89,11 @@
5 if not ids:
6 return 0 if count else []
7
8+ # Work with a set, as list.remove() is prohibitive for large lists of documents
9+ # (takes 20+ seconds on a db with 100k docs during search_count()!)
10+ orig_ids = ids
11+ ids = set(ids)
12+
13 # Filter out documents that are in directories that the user is not allowed to read.
14 # Must use pure SQL to avoid access rules exceptions (we want to remove the records,
15 # not fail), and the records have been filtered in parent's search() anyway.
16@@ -108,7 +113,8 @@
17 for parent_id in visible_parent_ids:
18 ids.extend(parents[parent_id])
19
20- return len(ids) if count else ids
21+ result = [id for id in orig_ids if id in ids]
22+ return len(result) if count else result
23
24 def copy(self, cr, uid, id, default=None, context=None):
25 if not default: