many2one function fields are recomputed every time even with store=True

Hello Sébastien,

Thanks for the report.

Investigating it more made us face that there is a contradiction over the behavior of many2one with store=True and call of read().

We found that bug 594504 is a complete opposite to what has been said here.

We prefer to have a discussion with framework experts about the intended behavior in order to justify both the needs whether to call the method or not.

You may share your views here.

Thanks.

Hello Rifakat.
In my opinion when a function field with many2one have the option store=True, the orm should read the id from the database and after link it to the object related. And if the option store=False is set it should always read it from the function.
Moreover In my case I also use the inv_fnc to set the value.
Indeed depending of the context my field is in readonly or not, and so the user can filled the field manually so I have to read it from the database because I can not compute it as it was set manually.

You can check the code where I used it http://bazaar.launchpad.net/~akretion-team/openobject-addons/base_external_ref_error_managment/revision/5485
in the file sale

Extract of the code :

    def _get_referential_id(self, cr, uid, ids, name, args, context=None):
        res = {}
        for shop in self.browse(cr, uid, ids, context=context):
            if shop.shop_group_id:
                res[shop.id] = shop.shop_group_id.referential_id.id
            else:
                #path to fix orm bug indeed even if function field are store, the value is never read for many2one fields
                cr.execute('select referential_id from sale_shop where id=%s', (shop.id,))
                result = cr.fetchone()
                res[shop.id] = result[0]
        return res

    def _set_referential_id(self, cr, uid, id, name, value, arg, context=None):
        shop = self.browse(cr, uid, id, context=context)
        if shop.shop_group_id:
            raise osv.except_osv(_("User Error"), _("You can not change the referential of this shop, please change the referential of the shop group!"))
        else:
            if value == False:
                cr.execute('update sale_shop set referential_id = NULL where id=%s', (id,))
            else:
                cr.execute('update sale_shop set referential_id = %s where id=%s', (value, id))
        return True

    def _get_shop_id(self, cr, uid, ids, context=None):
        shop_ids=[]
        for group in self.pool.get('external.shop.group').browse(cr, uid, ids, context=context):
            shop_ids.append([shop.id for shop in group.shop_ids])
        return shop_ids

        'referential_id': fields.function(_get_referential_id, fnct_inv = _set_referential_id, type='many2one',
                relation='external.referential', string='External Referential', method=True,
                store={
                    'external.shop.group': (_get_shop_id, ['referential_id'], 10),
                 }),

As you can see I had an ugly patch to force to read into the database the value of the id when the function is call.
In the case that fnct_inv is used with store=True, we don't have the choice the orm have to read it the value from the database because the value can not be computed.

Hope my explication can help you.

Best Regards

please add this to maintanance

we must be able to rely that these basics work

I am still having the bug with an up-to-date trunk version. This kind of basic framework bug is not critical, but it should really be fixed !

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?

Hi Olivier, sorry for responding so late (too much mail in my mail box :S)
Thank for your anwser.
I think option 2 is reasonable for the moment and will work for me.

Regarding my behaviour I know it's kind of strange to have a different result If I use the result in the store field or if I compute it.
But In fact I need a field that sometime is used as a computed field and sometime use as a normal one2many field.
Indeed some e-commerce solution introduce the concept of shop_group and some not.
If I have a shop_group, the link between the shop and the referential is given by reading the value of the referential of the shop group. If I don't have any shop_group the user must set manually the referential in the shop.

One solution to avoid the problem will to store the value of the referential in an other field "direct_referential". So when I compute the field it will use :
- the value from the shop_group if a shop was set
OR
- the value in the field "direct_referential".

At the start I didn't choose that solution because I was thinking it's was a little useless to have two column in the database with the same value. And I was thinking to use the behaviour of storing as memory. But finally I think your right even if storing field works in some case this can introduce corrupted data, (for example if we launch a recomputation of stored field). I will think about it and refactor my code.

Thanks for your time.

Hi,

How to avoid re-compute function fields with store = {} or store = True? It affects performance too much. I rely on function fields for synchronizing with other fields in other classes. And I also rely on these stored values of function fields for doing sql query.

Currently I do some little trick to make syn between computed value and stored value. However, I face performance issue too much. In my table, it has about 150 records but it takes time about several seconds to show. This is due to the function for function fields is rather heavy processing.

So, please give me some ideas.

Thank you too much,

server info: openerp-server 6.1, latest rev