Comment 5 for bug 821583

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote : Re: function field type many2one and store=True doesn't work

Hi Sebastien,

First of all, I'd like to point out that this is really only a performance optimization, but should not affect the correctness of the results. It seems you describe a situation where you rely on the stored value because the computation of the function field would not provide a correct result - this is inconsistent and unsupported. The fnct_inv function must be the reverse of the fnct function if it is provided, otherwise you're allowing it to somehow "corrupt" the data. Even if that suits your business needs, that does not seem like a good practice, and the system should not guarantee that it will work.

As for the issue of re-computing stored many2one function fields all the time, this is a limitation of the current OpenERP framework, and would happen for any function fields that can be stored but do require a form of postprocessing (_classic_read=False). The only field type with these properties is currently many2one.
What happens it the following: the store parameter is not ignored, but is only used for the initial fetch from the database. Then because the field requires post-processing the value is passed to the get() method of the field.
For a regular many2one field, get() will turn the raw ID values coming from the database into "name_get" pair (ID, Label).
However for function fields there is a conflict: the get() method directly delegates to the `fnct` function implementing the function field, and its signature does not include possible prefetched values, so the prefetched values are ignored.

I see two options:

1. We modify the signature of function fields to include an optional 'stored_values' parameter that may contain the currently stored value for the field, if the field is stored. That is an API change and short of doing dirty tricks to introspect the functions before calling them, we'd have to update all function fields. We could of course do another dirty hack and pass the 'stored_values' in the context, but in order to benefit from it we'd still need to change the implementation of function fields to properly use it.
So that's only a long-term option.

2. Or we could add some logic to the current get() method of fields.function to check the contents of the prefetched 'values' before calling the function implementation, and only call it for the missing values. We won't be able to distinguish NULL values from values that have never been computed/stored yet, so we'll have to compute them anyway, but we'll still get the benefit of not computing the non-NULL values. This requires no change of API, but will not give function fields any control on how stored values are used. This is a short-term option.

I think option 2 is reasonable, and pretty much consistent with the current API convention: function fields are not supposed to know anything about their possible storage, only how to compute the result.

What do you think?