Searching on float fields does not work properly

Bug #634634 reported by Albert Cervera i Areny - http://www.NaN-tic.com
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Odoo Server (MOVED TO GITHUB)
Confirmed
Wishlist
OpenERP's Framework R&D

Bug Description

Trying to search with [('float_field','=',3.28)] or [('float_field','=','3.28')] may not work correctly because OpenERP converts the value in the right into float before doing the search in the database. This is an issue because it means that one cannot relay on the search, even if the field in the database is NUMERIC instead of FLOAT. The reason is that the final query may end up being:

WHERE float_field = 3.2800012

which will not exist.

The attached patch, checks if field is of type float and allows [('float_field','=','3.28')] to be used and thus allow searching for exact numbers.

Revision history for this message
Albert Cervera i Areny - http://www.NaN-tic.com (albert-nan) wrote :
Revision history for this message
xrg (xrg) wrote : Re: [Bug 634634] Re: Searching on float fields does not work properly

On Friday 10 September 2010, you wrote:
> ** Patch added: "expression.py.diff"
>
> https://bugs.launchpad.net/bugs/634634/+attachment/1563966/+files/expressi
> on.py.diff

Your patch is not wrong, but it makes me wonder:

why do we have the _symbol_c and _symbol_f like that now?
Why do we default to a 0.0 float, when we could even have NULL values?

I guess, blindly changing them might break *a lot* of assumptions in modules,
but shouldn't we think of null values and also perform less conversions?

Revision history for this message
Albert Cervera i Areny - http://www.NaN-tic.com (albert-nan) wrote :

A Divendres, 10 de setembre de 2010, xrg va escriure:
> On Friday 10 September 2010, you wrote:
> > ** Patch added: "expression.py.diff"
> >
> > https://bugs.launchpad.net/bugs/634634/+attachment/1563966/+files/express
> > i on.py.diff
>
> Your patch is not wrong, but it makes me wonder:
>
> why do we have the _symbol_c and _symbol_f like that now?
> Why do we default to a 0.0 float, when we could even have NULL values?

Your're right about that.

>
> I guess, blindly changing them might break *a lot* of assumptions in
> modules, but shouldn't we think of null values and also perform less
> conversions?

OpenERP does not distinguish between 0.0 and NULL in numeric fields. I think
there's a wish already somewhere by Ferdinand.

--
Albert Cervera i Areny
http://www.NaN-tic.com
OpenERP Partners
Mòbil: +34 669 40 40 18

http://twitter.com/albertnan
http://albert-nan.blogspot.com

Revision history for this message
Ferdinand (office-chricar) wrote : Re: [Openerp-expert-framework] [Bug 634634] Re: Searching on float fields does not work properly

On Friday 10 September 2010 P. Christeas wrote:
> On Friday 10 September 2010, you wrote:
> > ** Patch added: "expression.py.diff"
> >
> > https://bugs.launchpad.net/bugs/634634/+attachment/1563966/+files/expressi
> > on.py.diff
>
> Your patch is not wrong, but it makes me wonder:
>
> why do we have the _symbol_c and _symbol_f like that now?
> Why do we default to a 0.0 float, when we could even have NULL values?
>
> I guess, blindly changing them might break *a lot* of assumptions in
modules,
> but shouldn't we think of null values and also perform less conversions?
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-expert-framework
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~openerp-expert-framework
> More help : https://help.launchpad.net/ListHelp
>

I strongly vote for soring and showing Null values

the current way it is not possible to distinguish for Example between
0.0 °C Temperature and not data recorded
--
regards
Ferdinand Gassauer
ChriCar Beteiligungs- und Beratungs- GmbH
Official OpenERP Partner

Changed in openobject-server:
status: New → Confirmed
Revision history for this message
xrg (xrg) wrote : Re: [Bug 634634] Re: Searching on float fields does not work properly

On Friday 10 September 2010, you wrote:
> On Friday 10 September 2010, you wrote:
> > ** Patch added: "expression.py.diff"
> >
> > https://bugs.launchpad.net/bugs/634634/+attachment/1563966/+files/express
> > i on.py.diff
>
> Your patch is not wrong, but it makes me wonder:
>
> why do we have the _symbol_c and _symbol_f like that now?
> Why do we default to a 0.0 float, when we could even have NULL values?
>
> I guess, blindly changing them might break *a lot* of assumptions in
> modules, but shouldn't we think of null values and also perform less
> conversions?

I have "blindly" changed the symbol_set for integer, bigint, floats and am
running my server like that since 1-2 days. (commits, of course, are public in
my pg84 branch).

It surely breaks quite some code. (although I put a "compatibility" snippet
there).

I know this feature (allowing Null/None at numeric fields) has been desirable.
But it will need porting for many modules, it will have side-effects.

Changed in openobject-server:
importance: Undecided → Medium
assignee: nobody → OpenERP's Framework R&D (openerp-dev-framework)
Changed in openobject-server:
importance: Medium → Low
importance: Low → Wishlist
Revision history for this message
Nicolas Vanhoren (OpenERP) (niv-openerp) wrote :

Imprecision with floats is not to be considered as an issue, it's as normal behaviour due to the single precision floating-point format used in most modern programming languages. Allowing to specify a float as a string (or most other improvements that come to mind when trying to find a solution) is a feature that could be added to OpenERP but it would be too hard to test for V6. So it will be for a future version.

Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

@xrg,

may be a way to avoid side effects would be to define a new OpenERP float type that would allow differentiating Null/None.
Old float would still work the regular way, not breaking anything. Modules would be ported to the new float type as need arise.

@nicolas: I believe nobody here was talking about the float imprecision (float vs decimal I guess)

Revision history for this message
xrg (xrg) wrote :

On Tuesday 14 December 2010, you wrote:
> @xrg,
>
> may be a way to avoid side effects would be to define a new OpenERP float
> type that would allow differentiating Null/None.

Well, this is a good idea, but a bit late: I've already been running my branch
with the 'float null' patch for some time now, and only corrected one
regression so far. Said that, I'm still voting against any changes for v6.0
series; I'd rather not see any regressions at trunk now.

We can just schedule this change for v6.1, and fix all floats.

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.