Skip to content

Commit

Permalink
Merge pull request #4 from alexwiese/bugfix/cached-query
Browse files Browse the repository at this point in the history
Fix issues where query was being cached, moved modifications up to....
  • Loading branch information
alexwiese committed Dec 9, 2018
2 parents 400c95f + 400e81b commit 313f927
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 119 deletions.
4 changes: 2 additions & 2 deletions src/EFCore.OpenEdge/EFCore.OpenEdge.csproj
Expand Up @@ -21,8 +21,8 @@
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<WarningsAsErrors />
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<Version>1.0.3</Version>
<PackageReleaseNotes>Fix issue with parameters in SELECT list</PackageReleaseNotes>
<Version>1.0.4</Version>
<PackageReleaseNotes>Fix issues where query was being cached, moved modifications up to the model generator so that the actual query would differ and not be cached.</PackageReleaseNotes>
<AssemblyVersion>1.0.0.0</AssemblyVersion>
</PropertyGroup>

Expand Down
@@ -1,6 +1,7 @@
using EntityFrameworkCore.OpenEdge.Infrastructure.Internal;
using EntityFrameworkCore.OpenEdge.Metadata.Conventions.Internal;
using EntityFrameworkCore.OpenEdge.Query.ExpressionTranslators.Internal;
using EntityFrameworkCore.OpenEdge.Query.Internal;
using EntityFrameworkCore.OpenEdge.Query.Sql.Internal;
using EntityFrameworkCore.OpenEdge.Storage;
using EntityFrameworkCore.OpenEdge.Storage.Internal;
Expand All @@ -9,7 +10,9 @@
using EntityFrameworkCore.OpenEdge.Update.Internal;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Query.ExpressionTranslators;
using Microsoft.EntityFrameworkCore.Query.Internal;
using Microsoft.EntityFrameworkCore.Query.Sql;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Update;
Expand All @@ -31,6 +34,8 @@ public static IServiceCollection AddEntityFrameworkOpenEdge(this IServiceCollect
.TryAdd<ISingletonUpdateSqlGenerator, OpenEdgeUpdateSqlGenerator>()
.TryAdd<IModificationCommandBatchFactory, OpenEdgeModificationCommandBatchFactory>()
.TryAdd<IRelationalConnection>(p => p.GetService<IOpenEdgeRelationalConnection>())
.TryAdd<IRelationalResultOperatorHandler, OpenEdgeResultOperatorHandler>()
.TryAdd<IQueryModelGenerator, OpenEdgeQueryModelGenerator>()

.TryAdd<IBatchExecutor, BatchExecutor>()

Expand Down
@@ -0,0 +1,74 @@
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;
using Remotion.Linq.Parsing.ExpressionVisitors.TreeEvaluation;

namespace EntityFrameworkCore.OpenEdge.Query.ExpressionVisitors.Internal
{
public class OpenEdgeParameterExtractingExpressionVisitor : ParameterExtractingExpressionVisitor
{
public OpenEdgeParameterExtractingExpressionVisitor(IEvaluatableExpressionFilter evaluatableExpressionFilter,
IParameterValues parameterValues,
IDiagnosticsLogger<DbLoggerCategory.Query> logger,
DbContext context,
bool parameterize, bool
generateContextAccessors = false)
: base(evaluatableExpressionFilter, parameterValues, logger, context, parameterize, generateContextAccessors)
{
}

protected Expression VisitNewMember(MemberExpression memberExpression)
{
if (memberExpression.Expression is ConstantExpression constant
&& constant.Value != null)
{
switch (memberExpression.Member.MemberType)
{
case MemberTypes.Field:
return Expression.Constant(constant.Value.GetType().GetField(memberExpression.Member.Name).GetValue(constant.Value));

case MemberTypes.Property:
var propertyInfo = constant.Value.GetType().GetProperty(memberExpression.Member.Name);
if (propertyInfo == null)
{
break;
}

return Expression.Constant(propertyInfo.GetValue(constant.Value));
}
}

return base.VisitMember(memberExpression);
}

protected override Expression VisitNew(NewExpression node)
{
var memberArguments = node.Arguments.OfType<MemberExpression>().Select(VisitNewMember).ToList();

var arguments = Visit(node.Arguments.Where(a => !(a is MemberExpression)).ToList().AsReadOnly());

var newNode = node.Update(memberArguments.Concat(arguments));

return newNode;
}

protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
{
if (methodCallExpression.Method.Name == "Take")
{
return methodCallExpression;
}

if (methodCallExpression.Method.Name == "Skip")
{
return methodCallExpression;
}

return base.VisitMethodCall(methodCallExpression);
}
}
}
34 changes: 34 additions & 0 deletions src/EFCore.OpenEdge/Query/Internal/OpenEdgeQueryModelGenerator.cs
@@ -0,0 +1,34 @@
using System;
using System.Linq.Expressions;
using EntityFrameworkCore.OpenEdge.Query.ExpressionVisitors.Internal;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Query.Internal;
using Remotion.Linq.Parsing.ExpressionVisitors.TreeEvaluation;

namespace EntityFrameworkCore.OpenEdge.Query.Internal
{
public class OpenEdgeQueryModelGenerator : QueryModelGenerator
{
private readonly IEvaluatableExpressionFilter _evaluatableExpressionFilter;
private readonly ICurrentDbContext _currentDbContext;

public OpenEdgeQueryModelGenerator(INodeTypeProviderFactory nodeTypeProviderFactory,
IEvaluatableExpressionFilter evaluatableExpressionFilter,
ICurrentDbContext currentDbContext)
: base(nodeTypeProviderFactory, evaluatableExpressionFilter, currentDbContext)
{
_evaluatableExpressionFilter = evaluatableExpressionFilter;
_currentDbContext = currentDbContext;
}

public override Expression ExtractParameters(IDiagnosticsLogger<DbLoggerCategory.Query> logger, Expression query, IParameterValues parameterValues,
bool parameterize = true, bool generateContextAccessors = false)
{
return new OpenEdgeParameterExtractingExpressionVisitor(_evaluatableExpressionFilter, parameterValues, logger,
_currentDbContext.Context,
parameterize, generateContextAccessors).ExtractParameters(query);
}
}
}
@@ -0,0 +1,21 @@
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Query.Expressions;
using Microsoft.EntityFrameworkCore.Query.ExpressionVisitors;
using Microsoft.EntityFrameworkCore.Query.Internal;

namespace EntityFrameworkCore.OpenEdge.Query.Internal
{
public class OpenEdgeResultOperatorHandler : RelationalResultOperatorHandler
{
public OpenEdgeResultOperatorHandler(IModel model,
ISqlTranslatingExpressionVisitorFactory sqlTranslatingExpressionVisitorFactory,
ISelectExpressionFactory selectExpressionFactory,
IResultOperatorHandler resultOperatorHandler)
: base(model, sqlTranslatingExpressionVisitorFactory,
selectExpressionFactory, resultOperatorHandler)
{
}
}
}
118 changes: 1 addition & 117 deletions src/EFCore.OpenEdge/Query/Sql/Internal/OpenEdgeSqlGenerator.cs
@@ -1,4 +1,3 @@
using System;
using System.Linq;
using System.Linq.Expressions;
using EntityFrameworkCore.OpenEdge.Extensions;
Expand All @@ -16,7 +15,7 @@ public OpenEdgeSqlGenerator(QuerySqlGeneratorDependencies dependencies, SelectEx
: base(dependencies, selectExpression)
{
}

protected override Expression VisitParameter(ParameterExpression parameterExpression)
{
var parameterName = SqlGenerator.GenerateParameterName(parameterExpression.Name);
Expand Down Expand Up @@ -80,120 +79,5 @@ public override Expression VisitExists(ExistsExpression existsExpression)

return existsExpression;
}

protected override void GenerateTop(SelectExpression selectExpression)
{
if (selectExpression.Limit != null
&& selectExpression.Offset == null)
{
// OpenEdge doesn't allow braces around the limit
Sql.Append("TOP ");

if (selectExpression.Limit is ParameterExpression limitParameter
&& ParameterValues.TryGetValue(limitParameter.Name, out var value))
{
var typeMapping = Dependencies.TypeMappingSource.GetMapping(limitParameter.Type);

// OpenEdge does not support the user of parameters for TOP, so we use literal instead
Sql.Append(GenerateSqlLiteral(typeMapping, value));
}
else
{
Visit(selectExpression.Limit);
}

Sql.Append(" ");
}
}

protected override void GenerateProjection(Expression projection)
{
Expression RemoveParameters(Expression expression)
{
switch (expression)
{
case AliasExpression aliasExpression:
return new AliasExpression(aliasExpression.Alias, RemoveParameters(aliasExpression.Expression));

case ParameterExpression parameterExpression:
if (ParameterValues.TryGetValue(parameterExpression.Name, out var value))
{
return Expression.Constant(value);
}

return expression;

default:
return expression;
}
}

// OpenEdge doesn't allow parameters in a SELECT list
projection = RemoveParameters(projection);

base.GenerateProjection(projection);
}

protected override void GenerateLimitOffset(SelectExpression selectExpression)
{
if (selectExpression.Limit != null
&& selectExpression.Offset == null)
{
return;
}

var limit = selectExpression.Limit;
var offset = selectExpression.Offset;

// OpenEdge does not support the use of parameters for LIMIT/OFFSET
// Map the parameter expressions to constant expressions instead
if (selectExpression.Offset is ParameterExpression offsetParameter
&& ParameterValues.TryGetValue(offsetParameter.Name, out var value))
{
offset = Expression.Constant(value);
}

if (selectExpression.Limit is ParameterExpression limitParameter
&& ParameterValues.TryGetValue(limitParameter.Name, out value))
{
limit = Expression.Constant(value);
}

// Need to set limit to null first, to get around
// the push subquery logic in the setters
selectExpression.Limit = null;
selectExpression.Offset = null;

selectExpression.Offset = offset;
selectExpression.Limit = limit;

base.GenerateLimitOffset(selectExpression);
}

private string GenerateSqlLiteral(RelationalTypeMapping relationalTypeMapping, object value)
{
var mappingClrType = relationalTypeMapping?.ClrType.UnwrapNullableType();

if (mappingClrType != null
&& (value == null
|| mappingClrType.IsInstanceOfType(value)
|| value.GetType().IsInteger()
&& (mappingClrType.IsInteger()
|| mappingClrType.IsEnum)))
{
if (value?.GetType().IsInteger() == true
&& mappingClrType.IsEnum)
{
value = Enum.ToObject(mappingClrType, value);
}
}
else
{
relationalTypeMapping = Dependencies.TypeMappingSource.GetMappingForValue(value);
}

return relationalTypeMapping.GenerateSqlLiteral(value);
}

}
}

0 comments on commit 313f927

Please sign in to comment.