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

date() function is not translated #792

Open
jamerst opened this issue Jan 2, 2023 · 0 comments · May be fixed by #1407
Open

date() function is not translated #792

jamerst opened this issue Jan 2, 2023 · 0 comments · May be fixed by #1407
Assignees
Labels
bug Something isn't working

Comments

@jamerst
Copy link

jamerst commented Jan 2, 2023

Assemblies affected
ASP.NET Core OData 8.x

Describe the bug
The date() function is not translated correctly. It just returns the date itself instead of removing the time.

This is caused by the translation at

protected virtual Expression BindDate(SingleValueFunctionCallNode node, QueryBinderContext context)
{
CheckArgumentNull(node, context, "date");
Expression[] arguments = BindArguments(node.Parameters, context);
// We should support DateTime & DateTimeOffset even though DateTime is not part of OData v4 Spec.
Contract.Assert(arguments.Length == 1 && ExpressionBinderHelper.IsDateOrOffset(arguments[0].Type));
// EF doesn't support new Date(int, int, int), also doesn't support other property access, for example DateTime.Date.
// Therefore, we just return the source (DateTime or DateTimeOffset).
return arguments[0];
}
and
private Expression BindDate(SingleValueFunctionCallNode node)
{
Contract.Assert("date" == node.Name);
Expression[] arguments = BindArguments(node.Parameters);
// We should support DateTime & DateTimeOffset even though DateTime is not part of OData v4 Spec.
Contract.Assert(arguments.Length == 1 && ExpressionBinderHelper.IsDateOrOffset(arguments[0].Type));
// EF doesn't support new Date(int, int, int), also doesn't support other property access, for example DateTime.Date.
// Therefore, we just return the source (DateTime or DateTimeOffset).
return arguments[0];
}

The comments imply that it is not translated because EF does not support translating the DateTime.Date/DateTimeOffset.Date properties, but this is no longer the case with EF Core. I have confirmed that querying using the Date property works as expected with EF Core 6+ on Postgres, SQL Server, MySQL and SQLite (probably works on older versions of EF Core too).

Reproduce steps
Run a query with $compute=date(someDateTime) as d and $select=d, the values returned for d still contain the time:
Response

Expected behavior
The time should be removed from the DateTime/DateTimeOffset when the date() function is used:
Expected response

Additional context
This is a fairly simple thing to fix, it's a simple translation:

protected virtual Expression BindDate(SingleValueFunctionCallNode node, QueryBinderContext context)
{
    CheckArgumentNull(node, context, "date");

    Expression[] arguments = BindArguments(node.Parameters, context);

    // We should support DateTime & DateTimeOffset even though DateTime is not part of OData v4 Spec.
    Contract.Assert(arguments.Length == 1 && ExpressionBinderHelper.IsDateOrOffset(arguments[0].Type));

    return Expression.Property(arguments[0], nameof(DateTime.Date));
}

The above translation works with EF Core, however, I'm not sure how this should be handled since it would be a breaking change and would break compatibility with EF6.

It should be possible to add EF6 compatibility using the DbFunctions.TruncateTime method, assuming there is a way to detect that the data provider is EF6? I'm not sure if the other EF6 database providers support this method though, I've only ever used it with SQL Server.

It would also be fine to make this a configurable option?

On a slightly unrelated note, I noticed that the binding is duplicated. It appears that some cases still use the binding in ExpressionBinderBase (e.g. for aggregation and compute) instead of the binders provided by Dependency Injection as for filter, select/expand and orderby. This is a bit annoying since it means that I can't override the binding to add a workaround for the issue. I'm not sure if a refactor of the aggregation and compute binders is planned, some comments suggest that it is?

@jamerst jamerst added the bug Something isn't working label Jan 2, 2023
jamerst added a commit to jamerst/AspNetCoreOData that referenced this issue Feb 4, 2025
@jamerst jamerst linked a pull request Feb 5, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants