Skip to content

Commit

Permalink
Enable nullable backing field pattern for store-generated defaults
Browse files Browse the repository at this point in the history
Fixes #15182

There is quite a fundamental change included here: the CLR type reported for a mapped property is now the backing field type, if it is known, not the property type. This is what allows a three states to be saved for a non-nullable property with a nullable backing field.

Perceptive readers will realize that this means that the CLR type of an `IProperty` can now change if the backing field is changed.

The most difficult case to handle seems to be properties backed by `object` fields. This likely isn't very common, but we had tests for it because of previous issues reported. One scenario is not working here--creating a shadow property FK by convention for a PK that is backed by an object field. I will file a bug for this.

Query changes are just what I needed to do to re-enable some tests to get coverage for this change. In particular, the type inference is not a universal solution, but unblocks important tests.
  • Loading branch information
ajcvickers committed May 27, 2019
1 parent 239ebcc commit 59d57f2
Show file tree
Hide file tree
Showing 57 changed files with 1,823 additions and 256 deletions.
11 changes: 10 additions & 1 deletion src/EFCore.InMemory/Query/Pipeline/InMemoryQueryExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,16 @@ public Expression GetProjectionExpression(ProjectionMember member)
var readValueExpression = (MethodCallExpression)projection;
var index = (int)((ConstantExpression)readValueExpression.Arguments[1]).Value;

return _valueBufferSlots[index];
var valueBufferSlotExpression = _valueBufferSlots[index];

var memberType = member.MemberType;
if (memberType != null
&& memberType != valueBufferSlotExpression.Type)
{
valueBufferSlotExpression = Convert(valueBufferSlotExpression, memberType);
}

return valueBufferSlotExpression;
}

public LambdaExpression GetScalarProjectionLambda()
Expand Down
10 changes: 9 additions & 1 deletion src/EFCore.InMemory/Query/Pipeline/Translator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,15 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
var entityType = entityShaper.EntityType;
var property = entityType.FindProperty((string)((ConstantExpression)methodCallExpression.Arguments[1]).Value);

return _inMemoryQueryExpression.BindProperty(entityShaper.ValueBufferExpression, property);
var boundPropertyExpression = _inMemoryQueryExpression.BindProperty(entityShaper.ValueBufferExpression, property);

if (boundPropertyExpression.Type
!= methodCallExpression.Type)
{
boundPropertyExpression = Expression.Convert(boundPropertyExpression, methodCallExpression.Type);
}

return boundPropertyExpression;
}
}

Expand Down
43 changes: 39 additions & 4 deletions src/EFCore.Relational/Query/Pipeline/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,22 +1,57 @@
// 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;
using System.Linq;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;

namespace Microsoft.EntityFrameworkCore.Relational.Query.Pipeline
{
public static class ExpressionExtensions
{
public static RelationalTypeMapping InferTypeMapping(params Expression[] expressions)
public static RelationalTypeMapping InferTypeMapping(
IValueConverterSelector valueConverterSelector,
params Expression[] expressions)
{
for (var i = 0; i < expressions.Length; i++)
{
if (expressions[i] is SqlExpression sql
&& sql.TypeMapping != null)
if (expressions[i] is SqlExpression sqlExpression)
{
return sql.TypeMapping;
var mapping = sqlExpression.TypeMapping;
if (mapping != null)
{
return mapping;
}

// This is to cover the cases of enums and char values being erased when using
// literals in the expression tree. It adds a new converter onto the type mapper
// that takes the literal and converts it back to the type that the type mapping
// is expecting.
// This code is temporary until more complete type inference is completed.
if (sqlExpression is SqlUnaryExpression sqlUnaryExpression
&& sqlUnaryExpression.OperatorType == ExpressionType.Convert)
{
var operandType = sqlUnaryExpression.Operand.Type.UnwrapNullableType();
mapping = InferTypeMapping(valueConverterSelector, sqlUnaryExpression.Operand);

if (mapping != null
&& (operandType.IsEnum
|| operandType == typeof(char)
|| mapping.Converter?.ProviderClrType == typeof(byte[])))
{
if (mapping.ClrType != sqlUnaryExpression.Type)
{
var converter = valueConverterSelector.Select(sqlUnaryExpression.Type, mapping.ClrType).ToList();

mapping = (RelationalTypeMapping)mapping.Clone(converter.First().Create());
}

return mapping;
}
}
}
}

Expand Down
16 changes: 11 additions & 5 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,22 @@
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;

namespace Microsoft.EntityFrameworkCore.Relational.Query.Pipeline
{
public class SqlExpressionFactory : ISqlExpressionFactory
{
private readonly IRelationalTypeMappingSource _typeMappingSource;
private readonly IValueConverterSelector _valueConverterSelector;
private readonly RelationalTypeMapping _boolTypeMapping;

public SqlExpressionFactory(IRelationalTypeMappingSource typeMappingSource)
public SqlExpressionFactory(
IRelationalTypeMappingSource typeMappingSource,
IValueConverterSelector valueConverterSelector)
{
_typeMappingSource = typeMappingSource;
_valueConverterSelector = valueConverterSelector;
_boolTypeMapping = typeMappingSource.FindMapping(typeof(bool));
}

Expand Down Expand Up @@ -75,8 +80,9 @@ public SqlExpression ApplyTypeMapping(SqlExpression sqlExpression, RelationalTyp
private SqlExpression ApplyTypeMappingOnLike(LikeExpression likeExpression)
{
var inferredTypeMapping = ExpressionExtensions.InferTypeMapping(
likeExpression.Match, likeExpression.Pattern, likeExpression.EscapeChar)
?? _typeMappingSource.FindMapping(likeExpression.Match.Type);
_valueConverterSelector,
likeExpression.Match, likeExpression.Pattern, likeExpression.EscapeChar)
?? _typeMappingSource.FindMapping(likeExpression.Match.Type);

return new LikeExpression(
ApplyTypeMapping(likeExpression.Match, inferredTypeMapping),
Expand Down Expand Up @@ -155,7 +161,7 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
case ExpressionType.LessThanOrEqual:
case ExpressionType.NotEqual:
{
inferredTypeMapping = ExpressionExtensions.InferTypeMapping(left, right)
inferredTypeMapping = ExpressionExtensions.InferTypeMapping(_valueConverterSelector, left, right)
?? _typeMappingSource.FindMapping(left.Type);
resultType = typeof(bool);
resultTypeMapping = _boolTypeMapping;
Expand All @@ -180,7 +186,7 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
case ExpressionType.And:
case ExpressionType.Or:
{
inferredTypeMapping = typeMapping ?? ExpressionExtensions.InferTypeMapping(left, right);
inferredTypeMapping = typeMapping ?? ExpressionExtensions.InferTypeMapping(_valueConverterSelector, left, right);
resultType = left.Type;
resultTypeMapping = inferredTypeMapping;
}
Expand Down
10 changes: 9 additions & 1 deletion src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,18 @@ public RelationalTypeMappingInfo(
public bool? IsRowVersion => _coreTypeMappingInfo.IsRowVersion;

/// <summary>
/// The CLR type in the model.
/// The CLR type used to store the property in the model. This may be the backing field type.
/// May be null if type information is conveyed via other means (e.g. the store name in a relational type mapping info)
/// </summary>
public Type ClrType => _coreTypeMappingInfo.ClrType;

/// <summary>
/// The CLR type of the property, which may be different from <see cref="ClrType"/> if a backing field of
/// a different type is used.
/// May be null if type information is conveyed via other means (e.g. the store name in a relational type mapping info)
/// </summary>
public Type DeclaredClrType => _coreTypeMappingInfo.DeclaredClrType;

/// <summary>
/// Returns a new <see cref="TypeMappingInfo" /> with the given converter applied.
/// </summary>
Expand Down
90 changes: 58 additions & 32 deletions src/EFCore.Relational/Storage/RelationalTypeMappingSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Generic;
using System.Reflection;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
Expand Down Expand Up @@ -126,54 +127,79 @@ private RelationalTypeMapping FindMappingWithConversion(
k =>
{
var (info, providerType, converter) = k;
var sourceType = info.ClrType;
var sourceDeclaredType = info.DeclaredClrType;
var needsConvertFromObject
= sourceType == typeof(object)
&& sourceDeclaredType != typeof(object);
var effectiveSourceType = needsConvertFromObject ? sourceDeclaredType : sourceType;
ValueConverterInfo fromObjectConverter = default;
var fromObjectMappingInfo = info;
if (needsConvertFromObject)
{
fromObjectConverter = Dependencies.ValueConverterSelector
.Select(typeof(object), sourceDeclaredType)
.Single();
fromObjectMappingInfo = info.WithConverter(fromObjectConverter);
}
var mapping = providerType == null
|| providerType == info.ClrType
? FindMapping(info)
|| providerType == effectiveSourceType
? FindMapping(fromObjectMappingInfo)
: null;
if (mapping == null)
if (mapping == null
&& effectiveSourceType != null)
{
var sourceType = info.ClrType;
if (sourceType != null)
foreach (var converterInfo in Dependencies
.ValueConverterSelector
.Select(effectiveSourceType, providerType))
{
foreach (var converterInfo in Dependencies
.ValueConverterSelector
.Select(sourceType, providerType))
{
var mappingInfoUsed = info.WithConverter(converterInfo);
mapping = FindMapping(mappingInfoUsed);
var mappingInfoUsed = info.WithConverter(converterInfo);
mapping = FindMapping(mappingInfoUsed);
if (mapping == null
&& providerType != null)
if (mapping == null
&& providerType != null)
{
foreach (var secondConverterInfo in Dependencies
.ValueConverterSelector
.Select(providerType))
{
foreach (var secondConverterInfo in Dependencies
.ValueConverterSelector
.Select(providerType))
{
mapping = FindMapping(mappingInfoUsed.WithConverter(secondConverterInfo));
mapping = FindMapping(mappingInfoUsed.WithConverter(secondConverterInfo));
if (mapping != null)
{
mapping = (RelationalTypeMapping)mapping.Clone(secondConverterInfo.Create());
break;
}
if (mapping != null)
{
mapping = (RelationalTypeMapping)mapping.Clone(secondConverterInfo.Create());
break;
}
}
}
if (mapping != null)
{
mapping = (RelationalTypeMapping)mapping.Clone(converterInfo.Create());
break;
}
if (mapping != null)
{
mapping = (RelationalTypeMapping)mapping.Clone(converterInfo.Create());
break;
}
}
}
if (mapping != null
&& converter != null)
if (mapping != null)
{
mapping = (RelationalTypeMapping)mapping.Clone(converter);
if (needsConvertFromObject)
{
mapping = (RelationalTypeMapping)mapping.Clone(fromObjectConverter.Create());
}
if (converter != null)
{
mapping = (RelationalTypeMapping)mapping.Clone(converter);
}
}
return mapping;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline;
using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using NetTopologySuite.Geometries;

namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Pipeline
Expand Down Expand Up @@ -49,13 +50,16 @@ public class SqlServerGeometryMethodTranslator : IMethodCallTranslator

private readonly IRelationalTypeMappingSource _typeMappingSource;
private readonly ISqlExpressionFactory _sqlExpressionFactory;
private readonly IValueConverterSelector _valueConverterSelector;

public SqlServerGeometryMethodTranslator(
IRelationalTypeMappingSource typeMappingSource,
ISqlExpressionFactory sqlExpressionFactory)
ISqlExpressionFactory sqlExpressionFactory,
IValueConverterSelector valueConverterSelector)
{
_typeMappingSource = typeMappingSource;
_sqlExpressionFactory = sqlExpressionFactory;
_valueConverterSelector = valueConverterSelector;
}

public SqlExpression Translate(SqlExpression instance, MethodInfo method, IList<SqlExpression> arguments)
Expand All @@ -64,7 +68,7 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IList<
{
var geometryExpressions = new[] { instance }.Concat(
arguments.Where(e => typeof(IGeometry).IsAssignableFrom(e.Type)));
var typeMapping = ExpressionExtensions.InferTypeMapping(geometryExpressions.ToArray());
var typeMapping = ExpressionExtensions.InferTypeMapping(_valueConverterSelector, geometryExpressions.ToArray());

Debug.Assert(typeMapping != null, "At least one argument must have typeMapping.");
var storeType = typeMapping.StoreType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.Extensions.DependencyInjection;

namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Pipeline
Expand All @@ -30,12 +31,14 @@ public class SqlServerNetTopologySuiteMethodCallTranslatorPlugin : IMethodCallTr
/// 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 SqlServerNetTopologySuiteMethodCallTranslatorPlugin(IRelationalTypeMappingSource typeMappingSource,
ISqlExpressionFactory sqlExpressionFactory)
public SqlServerNetTopologySuiteMethodCallTranslatorPlugin(
IRelationalTypeMappingSource typeMappingSource,
ISqlExpressionFactory sqlExpressionFactory,
IValueConverterSelector valueConverterSelector)
{
Translators = new IMethodCallTranslator[]
{
new SqlServerGeometryMethodTranslator(typeMappingSource, sqlExpressionFactory),
new SqlServerGeometryMethodTranslator(typeMappingSource, sqlExpressionFactory, valueConverterSelector),
new SqlServerGeometryCollectionMethodTranslator(typeMappingSource, sqlExpressionFactory),
new SqlServerLineStringMethodTranslator(typeMappingSource, sqlExpressionFactory),
new SqlServerPolygonMethodTranslator(typeMappingSource, sqlExpressionFactory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Reflection;
using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline;
using Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;

namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Pipeline
{
Expand Down Expand Up @@ -232,11 +233,14 @@ private readonly Dictionary<MethodInfo, string> _methodInfoDateDiffMapping
}
};
private readonly ISqlExpressionFactory _sqlExpressionFactory;
private readonly IValueConverterSelector _valueConverterSelector;

public SqlServerDateDiffFunctionsTranslator(
ISqlExpressionFactory sqlExpressionFactory)
ISqlExpressionFactory sqlExpressionFactory,
IValueConverterSelector valueConverterSelector)
{
_sqlExpressionFactory = sqlExpressionFactory;
_valueConverterSelector = valueConverterSelector;
}

public SqlExpression Translate(SqlExpression instance, MethodInfo method, IList<SqlExpression> arguments)
Expand All @@ -245,7 +249,7 @@ public SqlExpression Translate(SqlExpression instance, MethodInfo method, IList<
{
var startDate = arguments[1];
var endDate = arguments[2];
var typeMapping = ExpressionExtensions.InferTypeMapping(startDate, endDate);
var typeMapping = ExpressionExtensions.InferTypeMapping(_valueConverterSelector, startDate, endDate);

startDate = _sqlExpressionFactory.ApplyTypeMapping(startDate, typeMapping);
endDate = _sqlExpressionFactory.ApplyTypeMapping(endDate, typeMapping);
Expand Down
Loading

0 comments on commit 59d57f2

Please sign in to comment.