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

Query: Deprecate NullConditionalExpression #18686

Merged
merged 1 commit into from
Oct 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,6 @@ protected override Expression VisitExtension(Expression extensionExpression)
.GetMappedProjection(projectionBindingExpression.ProjectionMember)
: null;

case NullConditionalExpression nullConditionalExpression:
return Visit(nullConditionalExpression.AccessOperation);

default:
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -612,14 +612,6 @@ protected override Expression VisitExtension(Expression extensionExpression)
.GetMappedProjection(projectionBindingExpression.ProjectionMember)
: null;

case NullConditionalExpression nullConditionalExpression:
{
var translation = Visit(nullConditionalExpression.AccessOperation);
return translation.Type == nullConditionalExpression.Type
? translation
: Expression.Convert(translation, nullConditionalExpression.Type);
}

default:
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,9 +573,6 @@ protected override Expression VisitExtension(Expression extensionExpression)
case SqlExpression _:
return extensionExpression;

case NullConditionalExpression nullConditionalExpression:
return Visit(nullConditionalExpression.AccessOperation);

case EntityShaperExpression entityShaperExpression:
return Visit(entityShaperExpression.ValueBufferExpression);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -897,34 +897,13 @@ private Expression RewriteEntityEquality(
CreateKeyAccessExpression(Unwrap(right), keyProperties));
}

protected override Expression VisitExtension(Expression expression)
protected override Expression VisitExtension(Expression extensionExpression)
{
switch (expression)
{
case EntityReferenceExpression _:
// If the expression is an EntityReferenceExpression, simply returns it as all rewriting has already occurred.
// This is necessary when traversing wrapping expressions that have been injected into the lambda for parameters.
return expression;

case NullConditionalExpression nullConditionalExpression:
return VisitNullConditional(nullConditionalExpression);

default:
return base.VisitExtension(expression);
}
}

private Expression VisitNullConditional(NullConditionalExpression expression)
{
var newCaller = Visit(expression.Caller);
var newAccessOperation = Visit(expression.AccessOperation);
var visitedExpression = expression.Update(Unwrap(newCaller), Unwrap(newAccessOperation));

// TODO: Can the access operation be anything else than a MemberExpression?
return newCaller is EntityReferenceExpression wrapper
&& expression.AccessOperation is MemberExpression memberExpression
? wrapper.TraverseProperty(memberExpression.Member.Name, visitedExpression)
: visitedExpression;
// If the expression is an EntityReferenceExpression, simply returns it as all rewriting has already occurred.
// This is necessary when traversing wrapping expressions that have been injected into the lambda for parameters.
return extensionExpression is EntityReferenceExpression
? extensionExpression
: base.VisitExtension(extensionExpression);
}

private Expression CreateKeyAccessExpression(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ protected EntityReference UnwrapEntityReference(Expression expression)
case OwnedNavigationReference ownedNavigationReference:
return ownedNavigationReference.EntityReference;

case NullConditionalExpression nullConditionalExpression:
return UnwrapEntityReference(nullConditionalExpression.AccessOperation);

default:
return null;
}
Expand Down
27 changes: 1 addition & 26 deletions src/EFCore/Query/Internal/NullCheckRemovingExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ public class NullCheckRemovingExpressionVisitor : ExpressionVisitor
private readonly NullSafeAccessVerifyingExpressionVisitor _nullSafeAccessVerifyingExpressionVisitor
= new NullSafeAccessVerifyingExpressionVisitor();

private readonly NullConditionalRemovingExpressionVisitor _nullConditionalRemovingExpressionVisitor
= new NullConditionalRemovingExpressionVisitor();

protected override Expression VisitConditional(ConditionalExpression conditionalExpression)
{
var test = Visit(conditionalExpression.Test);
Expand All @@ -39,37 +36,15 @@ protected override Expression VisitConditional(ConditionalExpression conditional
? conditionalExpression.IfFalse
: conditionalExpression.IfTrue;

// Unwrap nested nullConditional
if (caller is NullConditionalExpression nullConditionalCaller)
{
accessOperation = ReplacingExpressionVisitor.Replace(
_nullConditionalRemovingExpressionVisitor.Visit(nullConditionalCaller.AccessOperation),
nullConditionalCaller,
accessOperation);
}

if (_nullSafeAccessVerifyingExpressionVisitor.Verify(caller, accessOperation))
{
return new NullConditionalExpression(caller, accessOperation);
return accessOperation;
}
}

return base.VisitConditional(conditionalExpression);
}

private class NullConditionalRemovingExpressionVisitor : ExpressionVisitor
{
public override Expression Visit(Expression expression)
{
if (expression is NullConditionalExpression nullConditionalExpression)
{
return Visit(nullConditionalExpression.AccessOperation);
}

return base.Visit(expression);
}
}

private class NullSafeAccessVerifyingExpressionVisitor : ExpressionVisitor
{
private readonly ISet<Expression> _nullSafeAccesses = new HashSet<Expression>(ExpressionEqualityComparer.Instance);
Expand Down
1 change: 1 addition & 0 deletions src/EFCore/Query/NullConditionalExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace Microsoft.EntityFrameworkCore.Query
/// Expression representing null-conditional access.
/// Logic in this file is based on https://github.com/bartdesmet/ExpressionFutures
/// </summary>
[Obsolete("Use ConditionalExpression with null check instead")]
smitpatel marked this conversation as resolved.
Show resolved Hide resolved
public class NullConditionalExpression : Expression, IPrintableExpression
{
/// <summary>
Expand Down
21 changes: 0 additions & 21 deletions test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -742,27 +742,6 @@ FROM root c
WHERE ((c[""Discriminator""] = ""Order"") AND (c[""CustomerID""] = ""FRANK""))");
}

public override async Task Null_conditional_simple(bool isAsync)
{
await base.Null_conditional_simple(isAsync);

AssertSql(
@"SELECT c
FROM root c
WHERE ((c[""Discriminator""] = ""Customer"") AND (c[""CustomerID""] = ""ALFKI""))");
}

[ConditionalTheory(Skip = "Issue #17246")]
public override async Task Null_conditional_deep(bool isAsync)
{
await base.Null_conditional_deep(isAsync);

AssertSql(
@"SELECT c
FROM root c
WHERE (c[""Discriminator""] = ""Customer"")");
}

public override async Task Queryable_simple(bool isAsync)
{
await base.Queryable_simple(isAsync);
Expand Down
46 changes: 0 additions & 46 deletions test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -518,52 +518,6 @@ public virtual Task Entity_equality_orderby_descending_composite_key(bool isAsyn
assertOrder: true);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Null_conditional_simple(bool isAsync)
{
var c = Expression.Parameter(typeof(Customer));

var predicate
= Expression.Lambda<Func<Customer, bool>>(
Expression.Equal(
new NullConditionalExpression(c, Expression.Property(c, "CustomerID")),
Expression.Constant("ALFKI")),
c);

return AssertQuery(
isAsync,
ss => ss.Set<Customer>().Where(predicate),
entryCount: 1);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Null_conditional_deep(bool isAsync)
{
var c = Expression.Parameter(typeof(Customer));

var nullConditionalExpression
= new NullConditionalExpression(c, Expression.Property(c, "CustomerID"));

nullConditionalExpression
= new NullConditionalExpression(
nullConditionalExpression,
Expression.Property(nullConditionalExpression, "Length"));

var predicate
= Expression.Lambda<Func<Customer, bool>>(
Expression.Equal(
nullConditionalExpression,
Expression.Constant(5, typeof(int?))),
c);

return AssertQuery(
isAsync,
ss => ss.Set<Customer>().Where(predicate),
entryCount: 91);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Queryable_simple(bool isAsync)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1234,26 +1234,6 @@ FROM [Orders] AS [o]
) AS [t0]");
}

public override async Task Null_conditional_simple(bool isAsync)
{
await base.Null_conditional_simple(isAsync);

AssertSql(
@"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
FROM [Customers] AS [c]
WHERE [c].[CustomerID] = N'ALFKI'");
}

public override async Task Null_conditional_deep(bool isAsync)
{
await base.Null_conditional_deep(isAsync);

AssertSql(
@"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
FROM [Customers] AS [c]
WHERE CAST(LEN([c].[CustomerID]) AS int) = 5");
}

public override async Task Queryable_simple(bool isAsync)
{
await base.Queryable_simple(isAsync);
Expand Down
13 changes: 2 additions & 11 deletions test/EFCore.Tests/Query/ExpressionPrinterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ public void Simple_MethodCall_printed_correctly()
public void Complex_MethodCall_printed_correctly()
{
Assert.Equal(
@"""Foobar"".Substring(
"\"Foobar\""
+ @".Substring(
startIndex: 0,
length: 4)",
_expressionPrinter.Print(
Expand Down Expand Up @@ -195,15 +196,5 @@ public void Linq_methods_printed_as_extensions()
.Where(x => x.Length > 1)",
_expressionPrinter.Print(expr.Body));
}

[ConditionalFact]
public void Use_Print_method_when_printing_extension_expression()
{
var expr = new NullConditionalExpression(
Expression.Constant("caller"),
Expression.Constant("accessOperation"));

Assert.Equal(expr.Print(), _expressionPrinter.Print(expr));
}
}
}