OData / OData/AspNetCoreOData

date() function is not translated

Open
#792 0 comments 3 reactions 1 assignee View on GitHub

@xuzhg is already working on this.

Since Jan 3, 2023.

bug
Dominant language
C#
Stars
505
Forks
186
PR merge metrics
No merged PRs in 30d

Description

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 https://github.com/OData/AspNetCoreOData/blob/4de92f52a346606a447ec4df96c5f3cd05642f50/src/Microsoft.AspNetCore.OData/Query/Expressions/QueryBinder.SingleValueFunctionCall.cs#L696-L708 and https://github.com/OData/AspNetCoreOData/blob/4de92f52a346606a447ec4df96c5f3cd05642f50/src/Microsoft.AspNetCore.OData/Query/Expressions/ExpressionBinderBase.cs#L404-L416

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?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.