trytond: don't try to detect rounding issue with MySQL (issue74005)
cedric...@b2ck.com
Please review this at http://codereview.tryton.org/74005/
Affected files:
M trytond/model/modelstorage.py
Index: trytond/model/modelstorage.py
===================================================================
--- a/trytond/model/modelstorage.py
+++ b/trytond/model/modelstorage.py
@@ -25,6 +25,7 @@
from trytond.transaction import Transaction
from trytond.pool import Pool
from trytond.cache import LRUDict
+from trytond.config import CONFIG
class ModelStorage(Model):
@@ -1108,7 +1109,7 @@
if not
(value.quantize(Decimal(str(10.0**-digits[1])))
== value):
raise_user_error()
- else:
+ elif CONFIG.options['db_type'] != 'mysql':
if not (round(value, digits[1]) == float(value)):
raise_user_error()
# validate digits
Cédric Krier
Even with this patch we still have testing issue with MySQL backend.
Indeed the problem is that MySQL has rounding issue with float and decimal.
For float, I can accept because it is float, but on decimal test like this
should not fail [1]
I think this makes MySQL impossible to work in production environment. So I
propose to drop the support of MySQL for the next version of Tryton. What do
you think?
[1]
http://hg.tryton.org/trytond/file/393318c8f257/trytond/tests/test_fields.py#l576
--
Cédric Krier
B2CK SPRL
Rue de Rotterdam, 4
4000 Liège
Belgium
Tel: +32 472 54 46 59
Email/Jabber: cedric...@b2ck.com
Website: http://www.b2ck.com/
Albert Cervera i Areny
A Dissabte, 6 d'agost de 2011 09:59:14, C�dric Krier va escriure:
> Hi,
>
> Even with this patch we still have testing issue with MySQL backend.
> Indeed the problem is that MySQL has rounding issue with float and decimal.
> For float, I can accept because it is float, but on decimal test like this
> should not fail [1]
> I think this makes MySQL impossible to work in production environment. So I
> propose to drop the support of MySQL for the next version of Tryton. What
> do you think?
I didn't try Tryton tests so I don't know exactly which of them is failing but did some tests manually and although MySQL's behaviour is pretty odd, I think it works correctly, at least when the search using decimal fields is in SQL. You can take a look at the log of the session I did (attached) to see when it works and when it does not.
The conclusion seems to be that one must cast using exactly the same definition as the field has. So if table creation looks like this:
create table test (a decimal);
Query must look like this:
select * from test where a=cast(1.1 as decimal);
If table creation looks like this:
create table test (a decimal(25,20));
Query must look like this:
select * from test where a=cast(1.1 as decimal(25,20));
Either that or simply cast both values:
select * from test where cast(a as decimal)=cast(1.1 as decimal);
It really sucks but it seems to work.
>
> [1]
> http://hg.tryton.org/trytond/file/393318c8f257/trytond/tests/test_fields.py
> #l576
--
Albert Cervera i Areny
Tel: +34 93 553 18 03
Cédric Krier
> A Dissabte, 6 d'agost de 2011 09:59:14, Cédric Krier va escriure:
> > Hi,
> >
> > Even with this patch we still have testing issue with MySQL backend.
> > Indeed the problem is that MySQL has rounding issue with float and decimal.
> > For float, I can accept because it is float, but on decimal test like this
> > should not fail [1]
> > I think this makes MySQL impossible to work in production environment. So I
> > propose to drop the support of MySQL for the next version of Tryton. What
> > do you think?
>
> I didn't try Tryton tests so I don't know exactly which of them is failing but
> did some tests manually and although MySQL's behaviour is pretty odd, I think
> it works correctly, at least when the search using decimal fields is in SQL.
> You can take a look at the log of the session I did (attached) to see when it
> works and when it does not.
I don't doubt that there should be solution to the issue but this is really
insane, the database should do the casting itself.
There is also an other issue with MySQL which is it doesn't store exactly the
float it receives. So you our test on digits [1] doesn't work.
Moreover the behavior of MySQL doesn't seem to be constant over versions.
Previous version was working correctly with comparison of decimal.
So I think the question is do we want to bloat Tryton (and spend Time) to be
able to support the strange behaviors of MySQL (per version)? Do we really
need MySQL support?
[1]
http://hg.tryton.org/trytond/file/393318c8f257/trytond/model/modelstorage.py#l1112
Albert Cervera i Areny
A Dissabte, 6 d'agost de 2011 13:30:31, C�dric Krier va escriure:
> So I think the question is do we want to bloat Tryton (and spend Time) to
> be able to support the strange behaviors of MySQL (per version)?
Me, definitely not.
> Do we really need MySQL support?
I don't think we'll ever use it so +1 for removing it.
alter...@gmail.com
Cédric Krier
> Could it be significant for integration with another software?
Not sure to understand your question.
I don't see how the DB could have an impact on the integration with an other
software.
Cédric Krier
> Hi,
>
> Even with this patch we still have testing issue with MySQL backend.
> Indeed the problem is that MySQL has rounding issue with float and decimal.
> For float, I can accept because it is float, but on decimal test like this
> should not fail [1]
> I think this makes MySQL impossible to work in production environment. So I
> propose to drop the support of MySQL for the next version of Tryton. What do
> you think?
I finnaly think we should keep the MySQL backend because the SQL service of
Google is based on MySQL.
Sasa Ostrouska
Huh, what a reason :) , I would say that the popularity of MySQL is
one of the reasons why you should keep that in Tryton, this makes
Tryton
way much easier to install and leave you a package out of installing a
whole bunch of dependencies. Just my point of view.
Although I would say that if MySQL has "technical" problems of
calculating the values the right way, then it should be visible to the
user somewhere.
Maybe pointing it out in the README with a big attention section.
Rgds
Saxa
Sasa Ostrouska
Ups, didn't noticed it.
Rgds
Saxa