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

DateTime converted to Nullable<DateTime> inside optional navigation property, throws exception on AddDays #6261

Closed
nth-commit opened this issue Aug 7, 2016 · 3 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@nth-commit
Copy link

nth-commit commented Aug 7, 2016

I believe this is a regression from RC1 or RC2 (I migrated from RC1 => RTM).

Trying to query all the orders completed within the last day:
(Note: Placement is the optional navigational property and, in the context of my solution, if it exists it indicates than the order has been placed)

var ordersCompleted = await dbContext.Orders
                .Where(o => o.IsCompleted)
                .Where(o => o.Error == OrderErrorType.None)
                .Where(o => o.Placement.Placed.AddDays(1) > now)
                .ToListAsync();

Exception:

Message: Method 'System.DateTime AddDays(Double)' declared on type 'System.DateTime' cannot be called with instance of type 'System.Nullable`1[System.DateTime]'

StackTrace: 
   at System.Linq.Expressions.Expression.ValidateCallInstanceType(Type instanceType, MethodInfo method)
   at System.Linq.Expressions.Expression.ValidateStaticOrInstanceMethod(Expression instance, MethodInfo method)
   at System.Linq.Expressions.Expression.Call(Expression instance, MethodInfo method, IEnumerable`1 arguments)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.VisitMethodCall(MethodCallExpression node)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.VisitBinary(BinaryExpression node)
   at Remotion.Linq.Clauses.WhereClause.TransformExpressions(Func`2 transformation)
   at Remotion.Linq.QueryModelVisitorBase.VisitBodyClauses(ObservableCollection`1 bodyClauses, QueryModel queryModel)
   at Remotion.Linq.QueryModelVisitorBase.VisitQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal.NavigationRewritingExpressionVisitor.Rewrite(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.OptimizeQueryModel(QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.EntityQueryModelVisitor.CreateAsyncQueryExecutor[TResult](QueryModel queryModel)
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddAsyncQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.System.Collections.Generic.IAsyncEnumerable<TResult>.GetEnumerator()
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.<ToListAsync>d__129`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at VegeRun.Services.Orders.Queries.OrderQueries.<GetLiveOrdersAsync>d__3.MoveNext() in C:\Users\Michael\Code\Personal\20160429\VegeRun\src\VegeRun.Services\Orders\Queries\OrderQueries.cs:line 155

Relevant classes (minimalized):

public class Order
{
public Guid Id { get; set; }

public bool IsCompleted { get; set; }

public OrderErrorType Error { get; set; }

public Guid? PlacementId { get; set; }

public virtual OrderPlacement Placement { get; set; }
}

public class OrderPlacement
{
public Guid Id { get; set; }

public DateTime Placed { get; set; }
}

Workaround:

var ordersCompleted2 = (await dbContext.Orders
                .Where(o => o.IsCompleted)
                .Where(o => o.Error == OrderErrorType.None)
                .Join(
                    dbContext.OrderPlacements,
                    o => o.PlacementId,
                    p => p.Id,
                    (o, p) => new { Order = o, Placement = p })
                .Where(op => op.Placement.Placed.AddDays(1) > now)
                .ToListAsync()).Select(op => op.Order);
@divega divega added this to the 1.1.0 milestone Aug 8, 2016
@divega divega added the type-bug label Aug 8, 2016
@divega
Copy link
Contributor

divega commented Aug 8, 2016

@maumar we belive this is happening because the nav prop is rewritten and the type becomes nullable.

@maumar
Copy link
Contributor

maumar commented Aug 8, 2016

Related to #5613

maumar added a commit that referenced this issue Aug 24, 2016
…c doesn't work for some cases involving multiple optional navs and method calls

Problem was that we were providing null protection in case of optional navigation access, but that did not include further calling methods on top of them.
If the navigation was null, the member access would return null and we would try to call a method on it, which results in NRE.

Fix is to introduce a new reducible expression that represents conditional access to a method or a member. In memory this gets rewritten to a Block expression that recursively evaluates it's caller, assigns it to a variable and depending on the value, either returns null or performs the required access operation. This is superior to using Conditional expression that we did before, because the caller only gets evaluated once, rather than N times.

Also fixes related issue #6261 - DateTime converted to Nullable<DateTime> inside optional navigation property, throws exception on AddDays.
maumar added a commit that referenced this issue Aug 24, 2016
…c doesn't work for some cases involving multiple optional navs and method calls

Problem was that we were providing null protection in case of optional navigation access, but that did not include further calling methods on top of them.
If the navigation was null, the member access would return null and we would try to call a method on it, which results in NRE.

Fix is to introduce a new reducible expression that represents conditional access to a method or a member. In memory this gets rewritten to a Block expression that recursively evaluates it's caller, assigns it to a variable and depending on the value, either returns null or performs the required access operation. This is superior to using Conditional expression that we did before, because the caller only gets evaluated once, rather than N times.

Also fixes related issue #6261 - DateTime converted to Nullable<DateTime> inside optional navigation property, throws exception on AddDays.
maumar added a commit that referenced this issue Aug 26, 2016
…c doesn't work for some cases involving multiple optional navs and method calls

Problem was that we were providing null protection in case of optional navigation access, but that did not include further calling methods on top of them.
If the navigation was null, the member access would return null and we would try to call a method on it, which results in NRE.

Fix is to introduce a new reducible expression that represents conditional access to a method or a member. In memory this gets rewritten to a Block expression that recursively evaluates it's caller, assigns it to a variable and depending on the value, either returns null or performs the required access operation. This is superior to using Conditional expression that we did before, because the caller only gets evaluated once, rather than N times.

Also fixes related issue #6261 - DateTime converted to Nullable<DateTime> inside optional navigation property, throws exception on AddDays.

CR: Andrew
maumar added a commit that referenced this issue Aug 26, 2016
…c doesn't work for some cases involving multiple optional navs and method calls

Problem was that we were providing null protection in case of optional navigation access, but that did not include further calling methods on top of them.
If the navigation was null, the member access would return null and we would try to call a method on it, which results in NRE.

Fix is to introduce a new reducible expression that represents conditional access to a method or a member. In memory this gets rewritten to a Block expression that recursively evaluates it's caller, assigns it to a variable and depending on the value, either returns null or performs the required access operation. This is superior to using Conditional expression that we did before, because the caller only gets evaluated once, rather than N times.

Also fixes related issue #6261 - DateTime converted to Nullable<DateTime> inside optional navigation property, throws exception on AddDays.

CR: Andrew
maumar added a commit that referenced this issue Aug 26, 2016
…c doesn't work for some cases involving multiple optional navs and method calls

Problem was that we were providing null protection in case of optional navigation access, but that did not include further calling methods on top of them.
If the navigation was null, the member access would return null and we would try to call a method on it, which results in NRE.

Fix is to introduce a new reducible expression that represents conditional access to a method or a member. In memory this gets rewritten to a Block expression that recursively evaluates it's caller, assigns it to a variable and depending on the value, either returns null or performs the required access operation. This is superior to using Conditional expression that we did before, because the caller only gets evaluated once, rather than N times.

Also fixes related issue #6261 - DateTime converted to Nullable<DateTime> inside optional navigation property, throws exception on AddDays.

CR: Andrew
@maumar
Copy link
Contributor

maumar commented Aug 26, 2016

fixed in 7472a67

@maumar maumar closed this as completed Aug 26, 2016
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 26, 2016
@ajcvickers ajcvickers modified the milestones: 1.1.0-preview1, 1.1.0 Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

4 participants