Nested $orderby in $select/$expand with built-in/custom function not working

[Xriuk]

Assemblies affected
ASP.NET Core OData 8.2.4

Describe the bug
When I try using a built-in or custom function in an $orderby expression inside a $select I get the following exception:

NotImplementedException
   in Microsoft.OData.UriParser.QueryNodeVisitor`1.Visit(ConvertNode nodeIn) in Microsoft.OData.UriParser\QueryNodeVisitor.cs: riga 54
   in Microsoft.OData.UriParser.ConvertNode.Accept[T](QueryNodeVisitor`1 visitor) in Microsoft.OData.UriParser\ConvertNode.cs: riga 28
   in Microsoft.AspNetCore.OData.Query.Validator.OrderByModelLimitationsValidator.Visit(SingleValueFunctionCallNode nodeIn) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\OrderByModelLimitationsValidator.cs: riga 142
   in Microsoft.OData.UriParser.SingleValueFunctionCallNode.Accept[T](QueryNodeVisitor`1 visitor) in Microsoft.OData.UriParser\SingleValueFunctionCallNode.cs: riga 55
   in Microsoft.AspNetCore.OData.Query.Validator.OrderByModelLimitationsValidator.TryValidate(OrderByClause orderByClause, Boolean explicitPropertiesDefined) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\OrderByModelLimitationsValidator.cs: riga 47
   in Microsoft.AspNetCore.OData.Query.Validator.OrderByModelLimitationsValidator.TryValidate(IEdmProperty property, IEdmStructuredType structuredType, OrderByClause orderByClause, Boolean explicitPropertiesDefined) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\OrderByModelLimitationsValidator.cs: riga 39
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidateNestedOrderby(OrderByClause orderByClause, SelectExpandValidatorContext validatorContext) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 405
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidatePathSelectItem(PathSelectItem pathSelectItem, SelectExpandValidatorContext validatorContext) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 303
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidateSelectExpand(SelectExpandClause selectExpandClause, SelectExpandValidatorContext validatorContext) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 102
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.Validate(SelectExpandQueryOption selectExpandQueryOption, ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 54
   in Microsoft.AspNetCore.OData.Query.SelectExpandQueryOption.Validate(ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Query\SelectExpandQueryOption.cs: riga 266
   in Microsoft.AspNetCore.OData.Query.Validator.ODataQueryValidator.Validate(ODataQueryOptions options, ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\ODataQueryValidator.cs: riga 106
   in Microsoft.AspNetCore.OData.Query.ODataQueryOptions.Validate(ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\ODataQueryOptions.cs: riga 652
...

Inside an $expand the exception is a bit different:

NotImplementedException
   in Microsoft.OData.UriParser.QueryNodeVisitor`1.Visit(ConvertNode nodeIn) in Microsoft.OData.UriParser\QueryNodeVisitor.cs: riga 54
   in Microsoft.OData.UriParser.ConvertNode.Accept[T](QueryNodeVisitor`1 visitor) in Microsoft.OData.UriParser\ConvertNode.cs: riga 28
   in Microsoft.AspNetCore.OData.Query.Validator.OrderByModelLimitationsValidator.Visit(SingleValueFunctionCallNode nodeIn) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\OrderByModelLimitationsValidator.cs: riga 142
   in Microsoft.OData.UriParser.SingleValueFunctionCallNode.Accept[T](QueryNodeVisitor`1 visitor) in Microsoft.OData.UriParser\SingleValueFunctionCallNode.cs: riga 55
   in Microsoft.AspNetCore.OData.Query.Validator.OrderByModelLimitationsValidator.TryValidate(OrderByClause orderByClause, Boolean explicitPropertiesDefined) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\OrderByModelLimitationsValidator.cs: riga 47
   in Microsoft.AspNetCore.OData.Query.Validator.OrderByModelLimitationsValidator.TryValidate(IEdmProperty property, IEdmStructuredType structuredType, OrderByClause orderByClause, Boolean explicitPropertiesDefined) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\OrderByModelLimitationsValidator.cs: riga 39
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidateNestedOrderby(OrderByClause orderByClause, SelectExpandValidatorContext validatorContext) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 405
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidateExpandedNavigationSelectItem(ExpandedNavigationSelectItem expandItem, SelectExpandValidatorContext validatorContext) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 200
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.ValidateSelectExpand(SelectExpandClause selectExpandClause, SelectExpandValidatorContext validatorContext) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 92
   in Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.Validate(SelectExpandQueryOption selectExpandQueryOption, ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\SelectExpandQueryValidator.cs: riga 54
   in Microsoft.AspNetCore.OData.Query.SelectExpandQueryOption.Validate(ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Query\SelectExpandQueryOption.cs: riga 266
   in Microsoft.AspNetCore.OData.Query.Validator.ODataQueryValidator.Validate(ODataQueryOptions options, ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\Validator\ODataQueryValidator.cs: riga 106
   in Microsoft.AspNetCore.OData.Query.ODataQueryOptions.Validate(ODataValidationSettings validationSettings) in D:\a\1\s\src\Microsoft.AspNetCore.OData\Query\ODataQueryOptions.cs: riga 652

Reproduce steps
A query like $select=CollectionProperty($orderby=length(Code)) or $expand=CollectionNavigation($orderby=length(Code))

[corranrogue9]

The correct syntax for this request is $select=CollectionProperty($orderby=length(code=Code)) (note the parameter name inside of the custom function parameter list). Also note that we have strict parsing logic around spacing (so, do not include spaces around the =).

That said, we need to improve this error message to indicate what the issue actually is, especially since callers will likely want to surface this as a 4xx to let their client know to fix the syntax.

[Xriuk]

I'm getting this exception now:

The argument for an invocation of a function with name 'length' is not a single value. All arguments for this function must be single values.
   at Microsoft.OData.UriParser.FunctionCallBinder.ValidateArgumentsAreSingleValue(String functionName, List`1 argumentNodes)
   at Microsoft.OData.UriParser.FunctionCallBinder.BindAsUriFunction(FunctionCallToken functionCallToken, List`1 argumentNodes)
   at Microsoft.OData.UriParser.FunctionCallBinder.BindFunctionCall(FunctionCallToken functionCallToken)
   at Microsoft.OData.UriParser.MetadataBinder.BindFunctionCall(FunctionCallToken functionCallToken)
   at Microsoft.OData.UriParser.MetadataBinder.Bind(QueryToken token)
   at Microsoft.OData.UriParser.OrderByBinder.ProcessSingleOrderBy(BindingState state, OrderByClause thenBy, OrderByToken orderByToken)
   at Microsoft.OData.UriParser.OrderByBinder.BindOrderBy(BindingState state, IEnumerable`1 orderByTokens)
   at Microsoft.OData.UriParser.SelectExpandBinder.BindOrderby(IEnumerable`1 orderByToken, IEdmNavigationSource resourcePathNavigationSource, IEdmNavigationSource targetNavigationSource, IEdmTypeReference elementType, HashSet`1 generatedProperties, Boolean collapsed)
   at Microsoft.OData.UriParser.SelectExpandBinder.GenerateSelectItem(SelectTermToken tokenIn)
   at Microsoft.OData.UriParser.SelectExpandBinder.Bind(ExpandToken expandToken, SelectToken selectToken)
   at Microsoft.OData.UriParser.SelectExpandSemanticBinder.Bind(ODataPathInfo odataPathInfo, ExpandToken expandToken, SelectToken selectToken, ODataUriParserConfiguration configuration, BindingState state)
   at Microsoft.OData.UriParser.ODataQueryOptionParser.ParseSelectAndExpandImplementation(String select, String expand, ODataUriParserConfiguration configuration, ODataPathInfo odataPathInfo)
   at Microsoft.OData.UriParser.ODataQueryOptionParser.ParseSelectAndExpand()
   at Microsoft.AspNetCore.OData.Query.SelectExpandQueryOption.get_SelectExpandClause()
   at Microsoft.AspNetCore.OData.Query.Validator.SelectExpandQueryValidator.Validate(SelectExpandQueryOption selectExpandQueryOption, ODataValidationSettings validationSettings)
   at Microsoft.AspNetCore.OData.Query.SelectExpandQueryOption.Validate(ODataValidationSettings validationSettings)
   at Microsoft.AspNetCore.OData.Query.Validator.ODataQueryValidator.Validate(ODataQueryOptions options, ODataValidationSettings validationSettings)
   at Microsoft.AspNetCore.OData.Query.ODataQueryOptions.Validate(ODataValidationSettings validationSettings)

In this case length is a built-in function, but the syntax you mentioned for custom functions (function(param1=value1,param2=value2)) is the same as the one for unbound functions in urls, why can't it be the same syntax used in $filter (function(value1,value2))? I don't see the specs specifying either (is it supported at all?) but it should be better for consistency, or support both.