Decimal number incorrectly parsed by the lexer

[raffaeler]

I have a Product entity exposed via oData with a "Test" property of type "decimal".
Among the others, the database contains these values (just the Test column):
3234567890.1234
1234.1234

Query 1
http://localhost:61320/odata/Products?$filter=Test%20eq%203234567890.1234M
Succeed because of the "M" suffix (even if it is deprecated by v4 of the specs)

Query 2
http://localhost:61320/odata/Products?$filter=Test%20eq%203234567890.1234
The query fails. The number is intepreted as a double instead of a decimal

Query 3
http://localhost:61320/odata/Products?$filter=Test%20eq%201234.1234
The query succeed but it is just for luck because the parser recognize the number as a float.

The faulty method is "MakeBestGuessOnNoSuffixStr":
https://github.com/OData/odata.net/blob/bfd173fdda4cd2f377316ba80c392979e291dfed/src/Microsoft.OData.Core/UriParser/ExpressionLexer.cs

The following screenshot shows the faulty method returning a double instead of a decimal.
In the watch window you can see the method parameters as well as the current instruction pointer.

The bug appears also in the latest (current) nuget package.
This is just a repro of the bug reported in production.

[chinadragon0515]

@raffaeler How to parse the number has been a long discussion, the conclusion we achieve is we parse the number to the lowest precision type which does not lose any precision of passed in number, in this way, and for consumer, he can cast lower precision type to higher precision type.

In the service logic of your code, you can cast the type to the type you need.

Let's know if you have any other questions or comments, thanks,

[raffaeler]

Casting would be the worst solution since doubles and floats are IEEE754 types which means loss of fidelity for the number.
You are now trying to undestand the type of the literal just by parsing it. This is IMHO wrong.
I would expect instead that you interpret the number assuming the type from the schema. In my example Test is a decimal and this is the parsing I expect.

The other solution is to apply the "M" suffix but the v4 specs explicitly make it obsolete, therefore the only possible safe solution is to derive the type looking at the schema.

[raffaeler]

Another reason for the cast being the wrong solution:
The queries I posted will never succeed because the literal in the $filter clause will never reach the controller's code.
The clause is resolved inside the oData code and the generated expression applied to the collection returned by the oData user controller.
Please take a look again to the samples I posted at the beginning of this thread.
Thank you

[chinadragon0515]

1. I checked the code and did some test here too, we have kept precision as full as possible,
   take example:
   a. For 203234567890.1234, if we use double, it will have value 203234567890.12341, and we will covert back to decimal and compare with decimal value from original string, we found double lose precision, we will use decimal.
   b. For 201234.1234, if we use float, it will become 201234.125, and double will keep precision, so we use double.
   c. For 3234567890.1234 (you case), double does not lose any precision here, but float will lose precision, so we use double.

   In our code, we already have logic to make sure the type we choose can assign to more precision type with exactly same value.

   We understand directly cast may lose precision, but it is not our case.

2. As "the literal in the $filter clause will never reach the controller's code.", do you mean filter does not take effect? Do you use entity framework? And there is a way for you to change the query option before it is applied, refer to class https://github.com/OData/ODataSamples/blob/master/WebApi/v4/ODataQueryableSample/Controllers/EmployeesController.cs, note GetFromManager method have ODataQueryOptions as parameter, you can modify the filter option there.

3. As to use type from model, I did more investigation, we can do some fix like we do for issue Uri parser can't work for short data type (Edm.Int16) for function parameter #154, but the class to fix this will be FilterBinder. Not sure whether you can help on this part.

[raffaeler]

The type deduction you are doing in the code is admirable but results also in spending a lot of time.
In my opinion the literal type can be deducted by the oData schema:
Say we have an Order with Quantity defined as decimal.
If the filter is quantity > 1234.5678, we can safely assume that 1234.5678 is a decimal too. Should it not fit in the decimal type (because the literal is too large), I would throw an exception

With regards to my example, the filter example I posted do not work (the predicate result in a false condition). In the sample project the provider is entity framework (not true for the real project).

In any case the solution to patch the literal number is not acceptable. It would mean to parse all the possible filters looking for literals. The real project is a framework, not a vertical custom solution, therefore the database is not known in advance.

Thanks

[chinadragon0515]

Move to backlog to see how the new reader and writer of OData 7.0 works for this.

[ElizabethOkerio]

This seems to be an issue in ASPNETCORE.ODATA and there is already another issue tracking it. Closing this.

[raffaeler]

@ElizabethOkerio I don't get why the other issue (opened just 23 days ago) was transferred and got "prevalence" over this one that is missing attention in 6 years (and still causes issues in large customer / corp.)
Frankly this is disturbing.