Merge lp:~tempo-openerp/openobject-server/keep_order_read-5.0 into lp:openobject-server/5.0

[Quentin THEURET @Amaris]

Fixes the #715749 bug

[Olivier Dony (Odoo) (odo-openerp)]

Quentin, thank you for the merge proposal, however we cannot accept such a change in stable branches, like 5.0 or 6.0.
You may want to re-submit against the trunk version only.

But even for trunk, this in-memory sorting of the results of read() can be a performance problem, and this is the reason why read() does not re-sort the result when it returns them.
Read() is basically a "SELECT [fields] from table where id in [ids]" which postgres has no reason to sort in the order of [ids]. To be able to do that, read() should receive the sort order to use, and thus pass it in the SELECT query.

This requires an API change, and in order to avoid duplicating the features of search() there, it would probably make sense to implement this as part of a new combined search_read() method, that would do both things at the same time, using postgres to implement the ordering efficiently.

Hope this helps.

review: Needs Resubmitting

[xrg (xrg)]

On Tuesday 15 March 2011, you wrote:
> Review: Resubmit
> This requires an API change, and in order to avoid duplicating the features
> of search() there, it would probably make sense to implement this as part
> of a new combined search_read() method, that would do both things at the
> same time, using postgres to implement the ordering efficiently.
>

Well, search_read() already does that, for one.
Remind me to write a test that will verify ordering of search_read() for the
next 100 years to come.

--
Say NO to spam and viruses. Stop using Microsoft Windows!

Unmerged revisions

2172. By Quentin THEURET <quentin@tempo-quentin> on 2011-02-09

[FIX] Keep the order of ids on the read ORM function

=== modified file 'bin/osv/orm.py'
--- bin/osv/orm.py	2011-02-04 09:32:33 +0000
+++ bin/osv/orm.py	2011-02-09 13:12:16 +0000
@@ -2358,7 +2358,7 @@
             for r in res:
                 for f in fields_post:
                     r[f] = self._columns[f]._symbol_get(r[f])
-        ids = map(lambda x: x['id'], res)
+        res = sorted(res, cmp=lambda x,y: cmp(ids.index(x['id']), ids.index(y['id'])))
 
         # all non inherited fields for which the attribute whose name is in load is False
         fields_post = filter(lambda x: x in self._columns and not getattr(self._columns[x], load), fields_to_read)