[6.0][5.0] ORM bug : function field with type one2many = "ir_translation polluted and duplicated function dead"

Bug #705364 reported by Sébastien BEAU - http://www.akretion.com
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Odoo Server (MOVED TO GITHUB)
Status tracked in Trunk
5.0
Won't Fix
Undecided
Unassigned
6.0
Won't Fix
Undecided
Unassigned
Trunk
Fix Committed
Low
OpenERP's Framework R&D

Bug Description

Hi !

I found a bug in the ORM on V5 and also on V6. To reproduce it, you need to install a module which has a function field with the type "one2many" ; this is the case of magentoerpconnect for example.

Scenario of the bug :
1) you must have two or more languages
2) you must have an object with a function field with the type "one2many" (in magentoerpconnect, the object sale_shop has a function field "exportable_product") and this function field must point to an object which has one or more translatable fields (in our example, the "name" field of the "product" object is translatable)
3) be sure that this field returns some ids
4) try to duplicate the object (in our example : try to duplicate a sale_shop)
5) you get the bug !

openerp V6 error:

[2011-01-20 09:47:53,947][demo_magento_v6] ERROR:web-services:Uncaught exception
Traceback (most recent call last):
  File "/home/sebastien/DEV/openerp/V6/server/bin/osv/osv.py", line 122, in wrapper
    return f(self, dbname, *args, **kwargs)
  File "/home/sebastien/DEV/openerp/V6/server/bin/osv/osv.py", line 176, in execute
    res = self.execute_cr(cr, uid, obj, method, *args, **kw)
  File "/home/sebastien/DEV/openerp/V6/server/bin/osv/osv.py", line 167, in execute_cr
    return getattr(object, method)(cr, uid, *args, **kw)
  File "/home/sebastien/DEV/openerp/V6/server/bin/osv/orm.py", line 4147, in copy
    self.copy_translations(cr, uid, id, new_id, context)
  File "/home/sebastien/DEV/openerp/V6/server/bin/osv/orm.py", line 4107, in copy_translations
    target_obj.copy_translations(cr, uid, old_child, new_child, context=context)
  File "/home/sebastien/DEV/openerp/V6/server/bin/osv/orm.py", line 4101, in copy_translations
    old_record, new_record = self.read(cr, uid, [old_id, new_id], [field_name], context=context)
ValueError: need more than 1 value to unpack

openerp V5 error:

Traceback (most recent call last):
  File "/home/sebastien/DEV/openerp/5.0/server/bin/netsvc.py", line 299, in dispatch
    result = LocalService(service_name)(method, *params)
  File "/home/sebastien/DEV/openerp/5.0/server/bin/netsvc.py", line 77, in __call__
    return getattr(self, method)(*params)
  File "/home/sebastien/DEV/openerp/5.0/server/bin/service/web_services.py", line 577, in execute
    res = service.execute(db, uid, object, method, *args)
  File "/home/sebastien/DEV/openerp/5.0/server/bin/osv/osv.py", line 58, in wrapper
    return f(self, dbname, *args, **kwargs)
  File "/home/sebastien/DEV/openerp/5.0/server/bin/osv/osv.py", line 119, in execute
    res = pool.execute_cr(cr, uid, obj, method, *args, **kw)
  File "/home/sebastien/DEV/openerp/5.0/server/bin/osv/osv.py", line 111, in execute_cr
    return getattr(object, method)(cr, uid, *args, **kw)
  File "/home/sebastien/DEV/openerp/5.0/server/bin/osv/orm.py", line 3211, in copy
    for record in translation_records:
  File "/home/sebastien/DEV/openerp/5.0/server/bin/osv/orm.py", line 3187, in copy_translations
    if field_def['type'] in ('one2one', 'one2many'):
  File "/home/sebastien/DEV/openerp/5.0/server/bin/osv/orm.py", line 3180, in copy_translations
ValueError: need more than 1 value to unpack

And now the ugly part ! By chance, Openerp doesn't succeed in copying the sale shop because the product is linked with bom_ids and in this case bom_ids=False ; therefore, the copy of the translation fails.

But, for the OpenERP install at Anevia, a more simple scenario : in one of the Anevia-specific modules, we added a field origin_country_ids on the picking which will return all country_id of the products present in the move lines. Country_id on the product is a translatable field.
In this case, Openerp succeeds in copying the translation of the country and this will start a TIME BOMB for your openerp server and your database (more details below).

OPENERP SHOULDN'T COPY THE DATA OF A FUNCTION FIELD! This must be fixed.

First bug (only present on V5) :
When you duplicate a sale_order, OpenERP will duplicate all the related sale order lines. The same way, when Anevia duplicates a picking, Openerp will try to duplicate all the country objects present in origin_country_ids ; but, by chance, as there is no inverse funtion, it can't!!

So the following patch will prevent the data to be copied in this scenario (this bug is already fixed in V6, but still unfixed in V5)

@@ -3132,7 +3132,7 @@

             if f in default:
                 data[f] = default[f]
- elif ftype == 'function':
+ elif '_fnct' in dir(self._columns[f]):
                 del data[f]
             elif ftype == 'many2one':
                 try:

In the code, the "ftype" variable will only contain "one2many" or "text" or "boolean" or ... but it will never contain "function" even if it is a function field.

Second bug (present in V5 and V6), which is the dangerous one : Openerp will copy all translations linked to the object selected by the function field (in our case openerp will duplicate all country translation ie : name)

So here is the TIME BOMB :
when you duplicate a picking for the first time : the table ir_translation will have 2 identical translations for the countries
when you duplicate a picking again with the same origin_coutrny_ids : the table ir_translation will have 4 identical translations for the countries
when you duplicate a picking again with the same origin_coutrny_ids : the table ir_translation will have 8 identical translations for the countries
when you duplicate a picking again with the same origin_coutrny_ids : the table ir_translation will have 16 identical translations for the countries
when you duplicate a picking again with the same origin_coutrny_ids : the table ir_translation will have 32 identical translations for the countries
..
..
..
..
after 20 duplicates : the table ir_translation will have 1 048 576 identical translations for the countries ! So, the next duplicate will copy 1 048 576 entries in the table ir_translation... so the OpenERP server and postgres database will eat all your CPU during a very very long time ! This is why this bug is a time bomb : you only start to experience it after some time of real-world use of your OpenERP.

This is the patch for this second bug :

@@ -3175,7 +3175,7 @@
         translation_records = []
         for field_name, field_def in fields.items():
             # we must recursively copy the translations for o2o and o2m
- if field_def['type'] in ('one2one', 'one2many'):
+ if field_def['type'] in ('one2one', 'one2many') and not '_fnct' in dir(self._columns[field_name]):
                 target_obj = self.pool.get(field_def['relation'])
                 old_record, new_record = self.read(cr, uid, [old_id, new_id], [field_name], context=context)
                 # here we rely on the order of the ids to match the translations

By the way, it would be good to add a constraint on the table ir_translation ; indeed, a field should only have one translation per object and per language. Adding a constraint would make sure that the ir_translation table never gets "polluted".

These 2 patches are already in production on Anevia's OpenERP v5 server.

After applying the patch, users should also clean-up the ir_translation table of their OpenERP database.

Best regards

Related branches

Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :
Changed in openobject-server:
importance: Undecided → Critical
Revision history for this message
xrg (xrg) wrote :

There is no "time bomb" string in the code. Perhaps you are talking about another project.

Revision history for this message
Alexis de Lattre (alexis-via) wrote :

No, this is a real bug on OpenERP server that we encountered on a real production deployment at Anevia. There is no "time bomb" string in the code of course ! :-) It's the bug in the ORM which is a time bomb.

Revision history for this message
xrg (xrg) wrote : Re: [Bug 705364] Re: [6.0][5.0] ORM bug : function field with type one2many = TIME BOMB

On Thursday 20 January 2011, you wrote:
> No, this is a real bug on OpenERP server that we encountered on a real
> production deployment at Anevia. There is no "time bomb" string in the
> code of course ! :-) It's the bug in the ORM which is a time bomb.
>

I'm just trying to turn LP into a Kindergarten, just like everybody else here
does.

For real bugs and serious conversation (without over-expressions) we might
need to find another medium.

summary: - [6.0][5.0] ORM bug : function field with type one2many = TIME BOMB
+ [6.0][5.0] ORM bug : function field with type one2many = "ir_translation
+ polluted and duplicated function dead"
Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

Sorry if the title looks too "hard". I change it ;) But be sure that this bug is real and also critical if you try to develop new feature for Openerp.

In order to help everybody to reproduce and understand this bug. I share this simple module "kill duplicate"

Scenario :
- install the module "kill duplicated" on Openerp V6
- install at least one translation
- create a picking
- duplicate the picking
- look in the database the country translation was useless copied

Now try to duplicate again and again your picking...

Your table ir_translation will be "polluted" and duplicate a picking will take some hour!! (and Openerp will eat all your CPU)

Best Regards

Revision history for this message
Fabien (Open ERP) (fp-tinyerp) wrote :

Sorry, I don't agree totally:

- OpenERP should be able to copy the data of a function field. (for instance if you have a write method defined or when it's a store=True field)
- the problem is that copy should not write function field IF they are no write methods defined.

Revision history for this message
Alexis de Lattre (alexis-via) wrote :

@xrg : I understand you were unhappy with the title we gave to the bug report. I can understand that. But why did you flag the bug as invalid instead of just changing the title ?

We had a very hard time finding and fixing this bug (cf patch proposed by Sebastien) ; our production OpenERP server was eating all the CPU of the server everytime we made a particular action which triggered a duplicate. You can call this kind of issue with other words, but it's still a "real" and serious bug.

Revision history for this message
xrg (xrg) wrote : Re: [Bug 705364] [NEW] [6.0][5.0] ORM bug : function field with type one2many = TIME BOMB

On Thursday 20 January 2011, Sébastien BEAU - http://www.akretion.com wrote:
> But, for the OpenERP install at Anevia, a more simple scenario : in one of
> the Anevia-specific modules, we added a field origin_country_ids on the
> picking which will return all country_id of the products present in the
> move lines. Country_id on the product is a translatable field. In this
> case, Openerp succeeds in copying the translation of the country and this
> will start a TIME BOMB for your openerp server and your database (more
> details below).

Please try the attached patch. It is an alternative solution, which hopefully
is more generic.

Revision history for this message
Yogesh (SerpentCS) (yogesh-serpentcs) wrote :

Hello,

Thanks for reporting.

It has been fixed at lp:~openerp-dev/openobject-server/trunk-bug-705364-ysa and it will be merged soon to the trunk server.

Thanks,

Revision history for this message
Christophe Combelles (ccomb) wrote :
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.