[trunk] [5.0] [6.0] keep ids order on read method with an _inherits object

[Quentin THEURET @Amaris]

On v.5 and v.6 trunk.

When you give a list of ids on read function of an _inherits object (like product.product), the read function returns automatically a list of dictionnaries sorted by id (for product.product) even if you give a not sorted list of id.

To see that, on a new db with demo data, create a XML-RPC script which search all products with 'pc' in their name. This search function returns you a list of ids sorted by id. Take this list of ids and sorted it again with the biggest id at first and the smallest at last.

Give this new sorted list to the read function and see the result of this function : the list of result is sorted by id (with the smallest id at first).

It's a bug because the read function doesn't keep the order of ids give to the function.

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Note:

I confirm this issue but thought it was on purpose (is that or
performance?). Notice that this caused us all sort of headaches with
magentoerpconnect (for instance you should respect some hierarchy order when
exporting categories to Magento). We worked around this in
base_external_referentials (extra addons) where de-defined a read method
that respect the order. If ever read unordered is significantly faster, then
may be you could provide a flag that would allow us to force the order to be
respected.

On Wed, Feb 9, 2011 at 12:46 PM, Quentin THEURET <email address hidden>wrote:

> ** Tags added: core object orm read
>
> --
> You received this bug notification because you are subscribed to OpenERP
> Server.
> https://bugs.launchpad.net/bugs/715749
>
> Title:
> [trunk] [5.0] [6.0] keep ids order on read method with an _inherits
> object
>
> Status in OpenERP Server:
> New
>
> Bug description:
> On v.5 and v.6 trunk.
>
> When you give a list of ids on read function of an _inherits object
> (like product.product), the read function returns automatically a list
> of dictionnaries sorted by id (for product.product) even if you give a
> not sorted list of id.
>
> To see that, on a new db with demo data, create a XML-RPC script which
> search all products with 'pc' in their name. This search function
> returns you a list of ids sorted by id. Take this list of ids and
> sorted it again with the biggest id at first and the smallest at last.
>
> Give this new sorted list to the read function and see the result of
> this function : the list of result is sorted by id (with the smallest
> id at first).
>
> It's a bug because the read function doesn't keep the order of ids
> give to the function.
>
>
>

[Quentin THEURET @Amaris (qtheuret)]

Hello Raphael,

I don't know if performances decrease with my modification. But before write this bug report, I also redefine the read method of the product.prodyct object but I wanted to know why the order wasn't kept.

If anyone can test with a lot of product to know if performances are bad with my patch.

I keep your remark about the flag and I will rewrite my patch with this solution when I'll have a moment.

[Quentin THEURET @Amaris (qtheuret)]

Indeed, after testing with 10,000 products, performance is very bad with my patch.

[Azazahmed Saiyed (OpenERP) (saz-openerp)]

Hello,

I have checked and issue remains there. I have attached the script which used for testing purpose at my end.

As the issue has not sever or affecting any of feature, I am setting this as a wishlist.

Thanks for your participation.

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

Just for the record, this is the comment I added on Quentin's merge proposals against 5.0 and 6.0, where he added in-memory sorting of read's result:

~~~
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.

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello Olivier and others,

so if that cost performance (which I understand), why not make this some
optional flag on the read method. So when you need it you have, but only
when you really need it?
At some point I don't find it normal everyone would have to re-implement
their own hacked sorted read in their custom module as we did in
base_external_referentials nor would I find it normal to suddenly add a perf
penalty to all read call while 99% of them don't need an ordered result.
Thoughts?

On Tue, Mar 15, 2011 at 11:13 AM, Olivier Dony (OpenERP) <
<email address hidden>> wrote:

> Just for the record, this is the comment I added on Quentin's merge
> proposals against 5.0 and 6.0, where he added in-memory sorting of
> read's result:
>
> ~~~
> 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.
>
> --
> You received this bug notification because you are subscribed to OpenERP
> Server.
> https://bugs.launchpad.net/bugs/715749
>
> Title:
> [trunk] [5.0] [6.0] keep ids order on read method with an _inherits
> object
>
> Status in OpenERP Server:
> Confirmed
>
> Bug description:
> On v.5 and v.6 trunk.
>
> When you give a list of ids on read function of an _inherits object
> (like product.product), the read function returns automatically a list
> of dictionnaries sorted by id (for product.product) even if you give a
> not sorted list of id.
>
> To see that, on a new db with demo data, create a XML-RPC script which
> search all products with 'pc' in their name. This search function
> returns you a list of ids sorted by id. Take this list of ids and
> sorted it again with the biggest id at first and the smallest at last.
>
> Give this new sorted list to the read function and see the result of
> this function : the list of result is sorted by id (with the smallest
> id at first).
>
> It's a bug because the read function doesn't keep the order of ids
> give to the function.
>

[Jozsef Vamosi (jozsef-vamosi)]

Hi

We had the same problem. When we search a model with a specified order close, the read method ignore the sorted id list the sort it with the default.
Because of sometimes we need a sorted result from read function we modified the _read_flat function with the following:

order_by = context['orderby'] or self._parent_order or self._order

So if we would like to use a different sorting method instead of the default just put it into the context and the red method will use that. If not, the read function works with the same way like before.

Regards
jozsef