Skip to content

Commit

Permalink
Fix to #18147 - Where bool column needs to convert to equality when v…
Browse files Browse the repository at this point in the history
…alue converter is applied

Added a postprocessor step that identifies bool columns with value converters inside a predicate and modifies them accordingly.

Fixes #18147
  • Loading branch information
maumar committed Jun 17, 2020
1 parent 60dc853 commit 5b7f684
Show file tree
Hide file tree
Showing 14 changed files with 553 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public static IServiceCollection AddEntityFrameworkCosmos([NotNull] this IServic
.TryAdd<ISingletonOptions, ICosmosSingletonOptions>(p => p.GetService<ICosmosSingletonOptions>())
.TryAdd<IQueryTranslationPreprocessorFactory, CosmosQueryTranslationPreprocessorFactory>()
.TryAdd<IQueryCompilationContextFactory, CosmosQueryCompilationContextFactory>()
.TryAdd<IQueryTranslationPostprocessorFactory, CosmosQueryTranslationPostprocessorFactory>()
.TryAddProviderSpecificServices(
b => b
.TryAddSingleton<ICosmosSingletonOptions, CosmosSingletonOptions>()
Expand Down
43 changes: 43 additions & 0 deletions src/EFCore.Cosmos/Query/CosmosQueryTranslationPostprocessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query
{
/// <inheritdoc />
public class CosmosQueryTranslationPostprocessor : QueryTranslationPostprocessor
{
private readonly ISqlExpressionFactory _sqlExpressionFactory;

/// <summary>
/// Creates a new instance of the <see cref="CosmosQueryTranslationPostprocessor" /> class.
/// </summary>
/// <param name="dependencies"> Parameter object containing dependencies for this class. </param>
/// <param name="sqlExpressionFactory"> The SqlExpressionFactory object to use. </param>
/// <param name="queryCompilationContext"> The query compilation context object to use. </param>
public CosmosQueryTranslationPostprocessor(
[NotNull] QueryTranslationPostprocessorDependencies dependencies,
[NotNull] ISqlExpressionFactory sqlExpressionFactory,
[NotNull] QueryCompilationContext queryCompilationContext)
: base(dependencies, queryCompilationContext)
{
Check.NotNull(sqlExpressionFactory, nameof(sqlExpressionFactory));

_sqlExpressionFactory = sqlExpressionFactory;
}

/// <inheritdoc />
public override Expression Process(Expression query)
{
query = base.Process(query);
query = new CosmosValueConverterCompensatingExpressionVisitor(_sqlExpressionFactory).Visit(query);

return query;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Utilities;
using Microsoft.Extensions.DependencyInjection;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
/// <summary>
/// <para>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </para>
/// <para>
/// The service lifetime is <see cref="ServiceLifetime.Singleton" />. This means a single instance
/// is used by many <see cref="DbContext" /> instances. The implementation must be thread-safe.
/// This service cannot depend on services registered as <see cref="ServiceLifetime.Scoped" />.
/// </para>
/// </summary>
public class CosmosQueryTranslationPostprocessorFactory : IQueryTranslationPostprocessorFactory
{
private readonly QueryTranslationPostprocessorDependencies _dependencies;
private readonly ISqlExpressionFactory _sqlExpressionFactory;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public CosmosQueryTranslationPostprocessorFactory(
[NotNull] QueryTranslationPostprocessorDependencies dependencies,
[NotNull] ISqlExpressionFactory sqlExpressionFactory)
{
_dependencies = dependencies;
_sqlExpressionFactory = sqlExpressionFactory;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual QueryTranslationPostprocessor Create(QueryCompilationContext queryCompilationContext)
{
Check.NotNull(queryCompilationContext, nameof(queryCompilationContext));

return new CosmosQueryTranslationPostprocessor(
_dependencies,
_sqlExpressionFactory,
queryCompilationContext);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
{
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public class CosmosValueConverterCompensatingExpressionVisitor : ExpressionVisitor
{
private readonly ISqlExpressionFactory _sqlExpressionFactory;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public CosmosValueConverterCompensatingExpressionVisitor(
[NotNull] ISqlExpressionFactory sqlExpressionFactory)
{
_sqlExpressionFactory = sqlExpressionFactory;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected override Expression VisitExtension(Expression extensionExpression)
{
return extensionExpression switch
{
ShapedQueryExpression shapedQueryExpression => VisitShapedQueryExpression(shapedQueryExpression),
ReadItemExpression readItemExpression => readItemExpression,
SelectExpression selectExpression => VisitSelect(selectExpression),
SqlConditionalExpression sqlConditionalExpression => VisitSqlConditional(sqlConditionalExpression),
_ => base.VisitExtension(extensionExpression),
};
}

private Expression VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression)
{
return shapedQueryExpression.Update(
Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression);
}

private Expression VisitSelect(SelectExpression selectExpression)
{
Check.NotNull(selectExpression, nameof(selectExpression));

var changed = false;

var projections = new List<ProjectionExpression>();
foreach (var item in selectExpression.Projection)
{
var updatedProjection = (ProjectionExpression)Visit(item);
projections.Add(updatedProjection);
changed |= updatedProjection != item;
}

var fromExpression = (RootReferenceExpression)Visit(selectExpression.FromExpression);
changed |= fromExpression != selectExpression.FromExpression;

var predicate = TryCompensateForBoolWithValueConverter(selectExpression.Predicate);
changed |= predicate != selectExpression.Predicate;

var orderings = new List<OrderingExpression>();
foreach (var ordering in selectExpression.Orderings)
{
var orderingExpression = (SqlExpression)Visit(ordering.Expression);
changed |= orderingExpression != ordering.Expression;
orderings.Add(ordering.Update(orderingExpression));
}

var limit = (SqlExpression)Visit(selectExpression.Limit);
var offset = (SqlExpression)Visit(selectExpression.Offset);

return changed
? selectExpression.Update(projections, fromExpression, predicate, orderings, limit, offset)
: selectExpression;
}

private Expression VisitSqlConditional(SqlConditionalExpression sqlConditionalExpression)
{
Check.NotNull(sqlConditionalExpression, nameof(sqlConditionalExpression));

var test = TryCompensateForBoolWithValueConverter(sqlConditionalExpression.Test);
var ifTrue = (SqlExpression)Visit(sqlConditionalExpression.IfTrue);
var ifFalse = (SqlExpression)Visit(sqlConditionalExpression.IfFalse);

return sqlConditionalExpression.Update(test, ifTrue, ifFalse);
}

private SqlExpression TryCompensateForBoolWithValueConverter(SqlExpression sqlExpression)
{
if (sqlExpression is KeyAccessExpression keyAccessExpression
&& keyAccessExpression.TypeMapping.ClrType == typeof(bool)
&& keyAccessExpression.TypeMapping.Converter != null)
{
return _sqlExpressionFactory.Equal(
sqlExpression,
_sqlExpressionFactory.Constant(true, sqlExpression.TypeMapping));
}

if (sqlExpression is SqlUnaryExpression sqlUnaryExpression)
{
return sqlUnaryExpression.Update(
TryCompensateForBoolWithValueConverter(sqlUnaryExpression.Operand));
}

if (sqlExpression is SqlBinaryExpression sqlBinaryExpression
&& (sqlBinaryExpression.OperatorType == ExpressionType.AndAlso
|| sqlBinaryExpression.OperatorType == ExpressionType.OrElse))
{
return sqlBinaryExpression.Update(
TryCompensateForBoolWithValueConverter(sqlBinaryExpression.Left),
TryCompensateForBoolWithValueConverter(sqlBinaryExpression.Right));
}

return sqlExpression;
}
}
}
43 changes: 43 additions & 0 deletions src/EFCore.Cosmos/Query/Internal/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ public SelectExpression(
_orderings = orderings;
}

private SelectExpression(
List<ProjectionExpression> projections,
RootReferenceExpression fromExpression,
List<OrderingExpression> orderings,
string container)
: this(projections, fromExpression, orderings)
{
Container = container;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down Expand Up @@ -495,5 +505,38 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)

return this;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual SelectExpression Update(
[NotNull] List<ProjectionExpression> projections,
[NotNull] RootReferenceExpression fromExpression,
[CanBeNull] SqlExpression predicate,
[CanBeNull] List<OrderingExpression> orderings,
[CanBeNull] SqlExpression limit,
[CanBeNull] SqlExpression offset)
{
Check.NotNull(projections, nameof(projections));
Check.NotNull(fromExpression, nameof(fromExpression));

var projectionMapping = new Dictionary<ProjectionMember, Expression>();
foreach (var kvp in _projectionMapping)
{
projectionMapping[kvp.Key] = kvp.Value;
}

return new SelectExpression(projections, fromExpression, orderings, Container)
{
_projectionMapping = projectionMapping,
Predicate = predicate,
Offset = offset,
Limit = limit,
IsDistinct = IsDistinct,
};
}
}
}
10 changes: 9 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/SqlExpressionVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
Expand All @@ -27,6 +28,13 @@ protected override Expression VisitExtension(Expression extensionExpression)

switch (extensionExpression)
{
case ShapedQueryExpression shapedQueryExpression:
return shapedQueryExpression.Update(
Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression);

case ReadItemExpression readItemExpression:
return readItemExpression;

case SelectExpression selectExpression:
return VisitSelect(selectExpression);

Expand Down Expand Up @@ -114,7 +122,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected abstract Expression VisitSqlConditional([NotNull] SqlConditionalExpression caseExpression);
protected abstract Expression VisitSqlConditional([NotNull] SqlConditionalExpression sqlConditionalExpression);

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Loading

0 comments on commit 5b7f684

Please sign in to comment.