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.

Sign up for GitHub

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

Enable nullable backing field pattern for store-generated defaults #15820

Closed
wants to merge 1 commit into from
Closed

Enable nullable backing field pattern for store-generated defaults #15820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion src/EFCore.InMemory/Query/Pipeline/InMemoryQueryExpression.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to the test which is failing so I can understand the type of tree encountered?

If this is dependent on a service then we can move the method to SqlExpressionFactory rather than extension method. Everyone using this method now having to take service looks ugly when they have already one service available.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the type mapping tests--see my comment. Keep in mind that this is just to unblock tests. I fully expect it to change in the future when we have working type inference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead with this then.

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
8 changes: 6 additions & 2 deletions src/EFCore.SqlServer.NTS/Query/Pipeline/SqlServerGeometryMethodTranslator.cs
View file
Open in desktop
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
9 changes: 6 additions & 3 deletions ...FCore.SqlServer.NTS/Query/Pipeline/SqlServerNetTopologySuiteMethodCallTranslatorPlugin.cs
View file
Open in desktop
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
8 changes: 6 additions & 2 deletions src/EFCore.SqlServer/Query/Pipeline/SqlServerDateDiffFunctionsTranslator.cs
View file
Open in desktop
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