ORM Constraint check

[David Lefever @ Taktik]

Hello,

I noticed a bug in the constraint checking system, in orm.py, method _add_sql_constraints around line 3380 :

elif unify_cons_text(con) not in [unify_cons_text(item['condef']) for item in existing_constraints]:

this cause problems because when creating a constraint in PostgreSQL, the DBMS can slightly modify it (to correct indentation for instance).

Here is an example:
Imagine in res_users I add a constraint unique(login,name)

If you paid attention, you see that there is no space between login,name.

Now, if I query the database like you do in orm.py around line 3394:

cr.execute("SELECT conname, pg_catalog.pg_get_constraintdef(oid, true) as condef FROM pg_constraint where conname=%s", (conname,))

you will get my constraint like this :

UNIQUE (login, name)

You can see that now, there is a space.

And that force the ERP to think the constraint changed, but I doesn't!
So every time I do a -u all, it drops and adds constraint that didn't change.
You can image that when the database is big, it can be a very long process.

Here are just some examples from our database :

2013-10-04 13:34:29,561 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'ir_model_fields': dropped constraint 'ir_model_fields_size_gt_zero'. Reason: its definition changed from 'check (size >= 0)' to 'CHECK (size>=0)'
2013-10-04 13:34:30,298 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'ir_rule': dropped constraint 'ir_rule_no_access_rights'. Reason: its definition changed from 'check (perm_read <> false or perm_write <> false or perm_create <> false or perm_unlink <> false)' to 'CHECK (perm_read!=False or perm_write!=False or perm_create!=False or perm_unlink!=False)'
2013-10-04 13:34:59,326 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'product_uom': dropped constraint 'product_uom_factor_gt_zero'. Reason: its definition changed from 'check (factor <> 0::numeric)' to 'CHECK (factor!=0)'
2013-10-04 13:35:07,654 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_model_line': dropped constraint 'account_model_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)'
2013-10-04 13:35:07,658 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_model_line': dropped constraint 'account_model_line_credit_debit2'. Reason: its definition changed from 'check ((credit + debit) >= 0::numeric)' to 'CHECK (credit+debit>=0)'
2013-10-04 13:35:08,533 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)'
2013-10-04 13:35:08,597 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit2'. Reason: its definition changed from 'check ((credit + debit) >= 0::numeric)' to 'CHECK (credit+debit>=0)'
2013-10-04 13:35:09,843 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_sequence_fiscalyear': dropped constraint 'account_sequence_fiscalyear_main_id'. Reason: its definition changed from 'check (sequence_main_id <> sequence_id)' to 'CHECK (sequence_main_id != sequence_id)'
2013-10-04 13:35:38,885 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'crm_lead': dropped constraint 'crm_lead_check_probability'. Reason: its definition changed from 'check (probability >= 0::double precision and probability <= 100::double precision)' to 'check(probability >= 0 and probability <= 100)'
2013-10-04 13:35:42,206 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'hr_holidays': dropped constraint 'hr_holidays_type_value'. Reason: its definition changed from 'check (holiday_type::text = 'employee'::text and employee_id is not null or holiday_type::text = 'category'::text and category_id is not null)' to 'CHECK( (holiday_type='employee' AND employee_id IS NOT NULL) or (holiday_type='category' AND category_id IS NOT NULL))'
2013-10-04 13:35:42,211 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'hr_holidays': dropped constraint 'hr_holidays_date_check2'. Reason: its definition changed from 'check (type::text = 'add'::text or date_from <= date_to)' to 'CHECK ( (type='add') OR (date_from <= date_to))'
2013-10-04 13:35:42,214 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'hr_holidays': dropped constraint 'hr_holidays_date_check'. Reason: its definition changed from 'check (number_of_days_temp >= 0::double precision)' to 'CHECK ( number_of_days_temp >= 0 )'
/Users/dvd/Taktik/openerp/lp/v7/openobject-server/openerp/osv/orm.py:839: UnicodeWarning: Unicode unequal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
  if cols[k][key] != vals[key]:
2013-10-04 13:35:44,498 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)'
2013-10-04 13:35:44,532 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit2'. Reason: its definition changed from 'check ((credit + debit) >= 0::numeric)' to 'CHECK (credit+debit>=0)'
2013-10-04 13:35:47,361 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)'
2013-10-04 13:35:47,395 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit2'. Reason: its definition changed from 'check ((credit + debit) >= 0::numeric)' to 'CHECK (credit+debit>=0)'
2013-10-04 13:36:01,380 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'crm_lead': dropped constraint 'crm_lead_check_probability'. Reason: its definition changed from 'check (probability >= 0::double precision and probability <= 100::double precision)' to 'check(probability >= 0 and probability <= 100)'
2013-10-04 13:36:12,099 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)'
2013-10-04 13:36:12,155 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit2'. Reason: its definition changed from 'check ((credit + debit) >= 0::numeric)' to 'CHECK (credit+debit>=0)'
2013-10-04 13:36:30,565 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'stock_warehouse_orderpoint': dropped constraint 'stock_warehouse_orderpoint_qty_multiple_check'. Reason: its definition changed from 'check (qty_multiple > 0)' to 'CHECK( qty_multiple > 0 )'
2013-10-04 13:36:36,147 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)'
2013-10-04 13:36:36,193 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit2'. Reason: its definition changed from 'check ((credit + debit) >= 0::numeric)' to 'CHECK (credit+debit>=0)'
2013-10-04 13:36:48,378 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)'
2013-10-04 13:36:48,418 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'account_move_line': dropped constraint 'account_move_line_credit_debit2'. Reason: its definition changed from 'check ((credit + debit) >= 0::numeric)' to 'CHECK (credit+debit>=0)'
2013-10-04 13:37:21,845 8548 DEBUG PROD_Taktik openerp.osv.orm.schema: Table 'crm_lead': dropped constraint 'crm_lead_check_probability'. Reason: its definition changed from 'check (probability >= 0::double precision and probability <= 100::double precision)' to 'check(probability >= 0 and probability <= 100)'

First line you can already see the problem :
Reason: its definition changed from 'check (size >= 0)' to 'CHECK (size>=0)'

Can you please check this?

I think a good solution would be to to store a hash of the constraints, and check against this hash.

[Rifakat Husen (OpenERP) (rha-openerp)]

Hi David,

I think is case is already taken care by a sub method of _add_sql_constraints(),
def unify_cons_text(txt):
    return txt.lower().replace(', ',',').replace(' (','(')

You are right that the constraint will be saved as UNIQUE (login, name) but when this line 3380 will be executed at that time it will lower case and remove space from the constraint and it will not raise any problem. Could you have a look at it please?

Please let me know if I have missed anything to trace it,
Rifakat

[David Lefever @ Taktik (dl-taktik)]

Hello, you are right but the unify_cons_text method does not cover all the cases.

For instance:
dropped constraint 'account_move_line_credit_debit1'. Reason: its definition changed from 'check ((credit * debit) = 0::numeric)' to 'CHECK (credit*debit=0)

I think the way of checking the constraints existing in Postgres and comparing them to the constraint written in the code is not a good solution, as nothing guarantees Postgres will not modify them.

[Rifakat Husen (OpenERP) (rha-openerp)]

Hi David,

Martin has created pull request for master,
https://github.com/odoo/odoo/pull/2409

Please have a look.