FilterExpression contains ConvertNode when using decimal with precision

[DickvdBrink]

Assemblies affected

OData .Net lib 6.15

Reproduce steps

        private static IEdmModel createModel()
        {
            EdmModel model = new EdmModel();
            EdmEntityContainer container = new EdmEntityContainer("", "name");
            model.AddElement(container);

            EdmEntityType type = new EdmEntityType("", "custom");

            var @decimal = EdmCoreModel.Instance.GetDecimal(6, 2, true);
            //var @decimal = EdmCoreModel.Instance.GetDecimal(true); with this code there isn't a convert node
            type.AddStructuralProperty("id", @decimal);
            container.AddEntitySet("custom", type);
            model.AddElement(type);

            return model;
        }

        static void Main(string[] args)
        {
            var model = createModel();
            Uri pathUri = new Uri("http://localhost/custom?$filter=id eq 4.2", UriKind.Absolute);
            ODataUriParser uriParser = new ODataUriParser(model, new Uri("http://localhost", UriKind.Absolute), pat

hUri);
            var result = uriParser.ParseUri();

// when using EdmCoreModel.Instance.GetDecimal(true); there isn't a convert node
// when using EdmCoreModel.Instance.GetDecimal(6, 2, true); the type of the constant is different from the id column
        }

See image below, I didn't expect the convertnode or at least I would have thougt the types of the constant to match the one from the id column.

If I use the code with GetDecimal(true); and change the 4.2 to an int (id eq 1) it promotes the constant to a decimal. I would have expected the same for the decimal with precision case

[Lingxi-Li]

Sorry, but I don't quite get your idea. Do you mean that the library should convert the decimal constant instead of the property, or the ConvertNode should not exist at all?

[DickvdBrink]

The convertnode should not exist at all (in my opinion) and the constantnode should have the same datatype (decimal with precision 6 and scale 2) as the SingleValuePropertyAccess node.

That would make the tree easier to use and it would match the behavior when using other datatypes (if I use strings there isn't a convertnode and if I use decimal without scale there isn't a convertnode either)

[Lingxi-Li]

Following is my thought on this:

1. Decimal with different precision, scale combinations should be different types, and explicit cast should be used to convert from one to another. This is based on the reasoning that values may not round-trip between decimal types with different precision, scale combinations.
2. According to the spec, when not explicitly specified, the decimal will have unspecified precision and zero scale. This should be the case for the decimal constant, which is different from the declared type of property id which has a precision and scale of 6 and 2, respectively.

Does this make sense?

[DickvdBrink]

It does make sense but I do have a question about 2.

Given the above model with this url: http://localhost/custom?$filter=id eq 4.2
The id column has decimal(6,2) but the value 4.2 is a constant - how do you give precision and scale there?

[Lingxi-Li]

According to what I have gathered, I'm afraid that, in the current V4 standard, there is no way to specify precision/scale for decimal literals in URLs. Rule primitiveLiteral in the OData-ABNF spec defines the representation of primitive type values in URLs (see last line):

;------------------------------------------------------------------------------
; 7. Literal Data Values
;------------------------------------------------------------------------------

; in URLs
primitiveLiteral = nullValue                  ; plain values up to int64Value
                 / booleanValue 
                 / guidValue 
                 / dateValue
                 / dateTimeOffsetValue 
                 / timeOfDayValue
                 / decimalValue 
                 ...

and

decimalValue = [SIGN] 1*DIGIT ["." 1*DIGIT]

I see no place for specifying precision/scale in the decimalValue rule, and hence my conclusion. @LaylaLiu @lewischeng-ms Could you confirm this limitation?

[Lingxi-Li]

Hi @DickvdBrink. I've discussed this issue with several experienced team members, and here are the results:

1. Currently, there is no way to specify precision/scale for decimal literals in URLs.
2. There are different ways to do the conversion, which produce different trees with different structures and involving different number of convert nodes. The spec says very little about how conversion should be done. As implementors, we cannot handle this ourselves. We will consult the standard committee for help.
3. We are going to factor the conversion logic using the strategy pattern. Consequently, users could specify customized conversion logic that best meets their needs. The library will provide a default logic based on the discussion results of 2.

[DickvdBrink]

Thanks for this! :) Will check this out when I have the time.
I'm running on .NET Core so it is a bit difficult to integrate the changes as I'm currently using a stripped version of OData WebApi.