Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$apply groupby fails when grouping more than one parameters #979

Closed
mirgil opened this issue Jul 3, 2023 · 6 comments · Fixed by #1014
Closed

$apply groupby fails when grouping more than one parameters #979

mirgil opened this issue Jul 3, 2023 · 6 comments · Fixed by #1014
Assignees
Labels
bug Something isn't working P2 regression

Comments

@mirgil
Copy link

mirgil commented Jul 3, 2023

Assemblies affected
Microsoft.AspNetCore.OData 8.0.12 & 8.2.0

Describe the bug
When providing 2 grouping parameters to the groupby, an exception is being thrown
$apply=groupby((Technology,Status))
Exception:
Instance property 'Values' is not defined for type 'System.Object' (Parameter 'propertyName')

Reproduce steps
Provide a $apply similar to the following one:
$apply=groupby((Technology,Status))

Data Model
Not relevant

EDM (CSDL) Model
Not relevant

Request/Response
Not relevant

Expected behavior
Same request with the same data model used to work on version 8.0.4.
Expect that it will work again.

Screenshots
Not relevant

Additional context
StackTrace:
at System.Linq.Expressions.Expression.Property(Expression expression, String propertyName)
at Microsoft.AspNetCore.OData.Query.Expressions.QueryBinder.GetPropertyExpression(Expression source, String propertyPath, Boolean isAggregated)
at Microsoft.AspNetCore.OData.Query.Expressions.QueryBinder.CreatePropertyAccessExpression(Expression source, QueryBinderContext context, IEdmProperty property, String propertyPath)
at Microsoft.AspNetCore.OData.Query.Expressions.QueryBinder.BindPropertyAccessQueryNode(SingleValuePropertyAccessNode propertyAccessNode, QueryBinderContext context)
at Microsoft.AspNetCore.OData.Query.Expressions.QueryBinder.BindSingleValueNode(SingleValueNode node, QueryBinderContext context)
at Microsoft.AspNetCore.OData.Query.Expressions.QueryBinder.Bind(QueryNode node, QueryBinderContext context)
at Microsoft.AspNetCore.OData.Query.Expressions.OrderByBinder.BindOrderBy(OrderByClause orderByClause, QueryBinderContext context)
at Microsoft.AspNetCore.OData.Query.Expressions.BinderExtensions.ApplyBind(IOrderByBinder binder, IQueryable query, OrderByClause orderByClause, QueryBinderContext context, Boolean alreadyOrdered)
at Microsoft.AspNetCore.OData.Query.OrderByQueryOption.AddOrderByQueryForProperty(IOrderByBinder orderByBinder, OrderByClause orderbyClause, IQueryable querySoFar, QueryBinderContext binderContext, Boolean alreadyOrdered)
at Microsoft.AspNetCore.OData.Query.OrderByQueryOption.ApplyToCore(IQueryable query, ODataQuerySettings querySettings)
at Microsoft.AspNetCore.OData.Query.OrderByQueryOption.ApplyTo(IQueryable query, ODataQuerySettings querySettings)
at Microsoft.AspNetCore.OData.Query.ODataQueryOptions.ApplyTo(IQueryable query, ODataQuerySettings querySettings)
at Microsoft.AspNetCore.OData.Query.ODataQueryOptions`1.ApplyTo(IQueryable query, ODataQuerySettings querySettings)
...

@mirgil mirgil added the bug Something isn't working label Jul 3, 2023
@julealgon
Copy link
Contributor

It's interesting that in the spec, it seems like a parenthesis is required.

Does it change the error if you do something like this?
$apply=groupby((Technology,Status))

@mirgil
Copy link
Author

mirgil commented Jul 4, 2023

It's interesting that in the spec, it seems like a parenthesis is required.

Does it change the error if you do something like this? $apply=groupby((Technology,Status))

You right, the sample I provided is missing the inner parenthesis - I fixed it in the description above. (The exception is thrown when providing the right syntax - if the inner parenthesis are missing, the exception is specific for the missing parenthesis)

@habbes
Copy link
Contributor

habbes commented Jul 4, 2023

This seems to be a regression. @ElizabethOkerio will investigate.

@ElizabethOkerio
Copy link
Contributor

@mirgil can you please share a repro for this. I'm not able to repro this issue. Thanks

@mirgil
Copy link
Author

mirgil commented Aug 6, 2023

@mirgil can you please share a repro for this. I'm not able to repro this issue. Thanks

I created the following repo:
https://github.com/mirgil/DefectsApp/

@ElizabethOkerio
Copy link
Contributor

Great. I'll check this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2 regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants