Skip to content

Commit

Permalink
Use configured default type mappings in ExecuteSql
Browse files Browse the repository at this point in the history
Fixes #32583
  • Loading branch information
AndriySvyryd committed Jan 23, 2024
1 parent e59d055 commit af13549
Show file tree
Hide file tree
Showing 23 changed files with 134 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public static int ExecuteSqlRaw(
try
{
var rawSqlCommand = facadeDependencies.RawSqlCommandBuilder
.Build(sql, parameters);
.Build(sql, parameters, databaseFacade.GetService<IModel>());

return rawSqlCommand
.RelationalCommand
Expand Down Expand Up @@ -613,7 +613,7 @@ public static async Task<int> ExecuteSqlRawAsync(
try
{
var rawSqlCommand = facadeDependencies.RawSqlCommandBuilder
.Build(sql, parameters);
.Build(sql, parameters, databaseFacade.GetService<IModel>());

return await rawSqlCommand
.RelationalCommand
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Metadata/Internal/RelationalModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ private static void CreateContainerColumn<TColumnMappingBase>(
{
Check.DebugAssert(tableBase.FindColumn(containerColumnName) == null, $"Table does not have column '{containerColumnName}'.");

var jsonColumnTypeMapping = relationalTypeMappingSource.FindMapping(typeof(JsonElement))!;
var jsonColumnTypeMapping = relationalTypeMappingSource.FindMapping(typeof(JsonElement), mappedType.Model)!;
var jsonColumn = createColumn(containerColumnName, tableBase, jsonColumnTypeMapping);
tableBase.Columns.Add(containerColumnName, jsonColumn);
jsonColumn.IsNullable = !ownership.IsRequiredDependent || !ownership.IsUnique;
Expand Down Expand Up @@ -1216,7 +1216,7 @@ static StoreStoredProcedure GetOrCreateStoreStoredProcedure(
storeStoredProcedure = new StoreStoredProcedure(storedProcedure.Name, storedProcedure.Schema, databaseModel);
if (storedProcedure.IsRowsAffectedReturned)
{
var typeMapping = relationalTypeMappingSource.FindMapping(typeof(int))!;
var typeMapping = relationalTypeMappingSource.FindMapping(typeof(int), databaseModel.Model)!;
storeStoredProcedure.ReturnValue = new StoreStoredProcedureReturnValue(
"",
typeMapping.StoreType,
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ private SqlExpression ApplyTypeMappingOnJsonScalar(

// The index expression isn't inferred and is always just an int. Apply the default type mapping to it.
var indexWithTypeMapping = ApplyDefaultTypeMapping(index);
var newPath = indexWithTypeMapping == index ? jsonScalarExpression.Path : new[] { new PathSegment(indexWithTypeMapping) };
var newPath = indexWithTypeMapping == index ? jsonScalarExpression.Path : [new PathSegment(indexWithTypeMapping)];

// If a type mapping is being applied from the outside, it applies to the element resulting from the array indexing operation;
// we can infer the array's type mapping from it. Otherwise there's nothing to do but apply the default type mapping to the array.
Expand All @@ -371,7 +371,7 @@ private SqlExpression ApplyTypeMappingOnJsonScalar(
ApplyDefaultTypeMapping(array),
newPath,
jsonScalarExpression.Type,
_typeMappingSource.FindMapping(jsonScalarExpression.Type),
_typeMappingSource.FindMapping(jsonScalarExpression.Type, Dependencies.Model),
jsonScalarExpression.IsNullable);
}

Expand Down Expand Up @@ -494,7 +494,7 @@ public virtual SqlFunctionExpression Coalesce(SqlExpression left, SqlExpression
typeMappedArguments,
nullable: true,
// COALESCE is handled separately since it's only nullable if *all* arguments are null
argumentsPropagateNullability: new[] { false, false },
argumentsPropagateNullability: [false, false],
resultType,
inferredTypeMapping);
}
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.Relational/Storage/IRawSqlCommandBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,16 @@ public interface IRawSqlCommandBuilder
RawSqlCommand Build(
string sql,
IEnumerable<object> parameters);

/// <summary>
/// Creates a new command based on SQL command text.
/// </summary>
/// <param name="sql">The command text.</param>
/// <param name="parameters">Parameters for the command.</param>
/// <param name="model">The model.</param>
/// <returns>The newly created command.</returns>
RawSqlCommand Build(
string sql,
IEnumerable<object> parameters,
IModel model);
}
17 changes: 15 additions & 2 deletions src/EFCore.Relational/Storage/Internal/RawSqlCommandBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ public virtual IRelationalCommand Build(string sql)
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual RawSqlCommand Build(string sql, IEnumerable<object> parameters)
=> Build(sql, parameters, null);

/// <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 RawSqlCommand Build(string sql, IEnumerable<object> parameters, IModel? model)
{
var relationalCommandBuilder = _relationalCommandBuilderFactory.Create();

Expand Down Expand Up @@ -81,8 +90,12 @@ public virtual RawSqlCommand Build(string sql, IEnumerable<object> parameters)

substitutions.Add(substitutedName);
var typeMapping = parameter == null
? _typeMappingSource.GetMappingForValue(null)
: _typeMappingSource.GetMapping(parameter.GetType());
? model == null
? _typeMappingSource.GetMappingForValue(null)
: _typeMappingSource.GetMappingForValue(null, model)
: model == null
? _typeMappingSource.GetMapping(parameter.GetType())
: _typeMappingSource.GetMapping(parameter.GetType(), model);
var nullable = parameter == null || parameter.GetType().IsNullableType();

relationalCommandBuilder.AddParameter(parameterName, substitutedName, typeMapping, nullable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,10 @@ public static RelationalTypeMapping GetMapping(

var mapping = typeMappingSource.FindMapping(property);

if (mapping != null)
{
return mapping;
}

throw new InvalidOperationException(
RelationalStrings.UnsupportedPropertyType(
property.DeclaringType.DisplayName(),
property.Name,
property.ClrType.ShortDisplayName()));
return mapping ?? throw new InvalidOperationException(RelationalStrings.UnsupportedPropertyType(
property.DeclaringType.DisplayName(),
property.Name,
property.ClrType.ShortDisplayName()));
}

/// <summary>
Expand All @@ -81,12 +75,7 @@ public static RelationalTypeMapping GetMapping(
Check.NotNull(clrType, nameof(clrType));

var mapping = typeMappingSource.FindMapping(clrType);
if (mapping != null)
{
return mapping;
}

throw new InvalidOperationException(RelationalStrings.UnsupportedType(clrType.ShortDisplayName()));
return mapping ?? throw new InvalidOperationException(RelationalStrings.UnsupportedType(clrType.ShortDisplayName()));
}

/// <summary>
Expand All @@ -104,12 +93,7 @@ public static RelationalTypeMapping GetMapping(
Check.NotNull(clrType, nameof(clrType));

var mapping = typeMappingSource.FindMapping(clrType, model);
if (mapping != null)
{
return mapping;
}

throw new InvalidOperationException(RelationalStrings.UnsupportedType(clrType.ShortDisplayName()));
return mapping ?? throw new InvalidOperationException(RelationalStrings.UnsupportedType(clrType.ShortDisplayName()));
}

/// <summary>
Expand All @@ -129,11 +113,6 @@ public static RelationalTypeMapping GetMapping(
Check.NotNull(typeName, nameof(typeName));

var mapping = typeMappingSource.FindMapping(typeName);
if (mapping != null)
{
return mapping;
}

throw new InvalidOperationException(RelationalStrings.UnsupportedStoreType(typeName));
return mapping ?? throw new InvalidOperationException(RelationalStrings.UnsupportedStoreType(typeName));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
namespace Microsoft.EntityFrameworkCore.Query;

public class NorthwindQueryCosmosFixture<TModelCustomizer> : NorthwindQueryFixtureBase<TModelCustomizer>
where TModelCustomizer : IModelCustomizer, new()
where TModelCustomizer : ITestModelCustomizer, new()
{
protected override ITestStoreFactory TestStoreFactory
=> CosmosNorthwindTestStoreFactory.Instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public class QueryLoggingCosmosTest(QueryLoggingCosmosTest.NorthwindQueryCosmosF
IClassFixture<QueryLoggingCosmosTest.NorthwindQueryCosmosFixtureInsensitive<NoopModelCustomizer>>
{
public class NorthwindQueryCosmosFixtureInsensitive<TModelCustomizer> : NorthwindQueryCosmosFixture<TModelCustomizer>
where TModelCustomizer : IModelCustomizer, new()
where TModelCustomizer : ITestModelCustomizer, new()
{
public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).EnableSensitiveDataLogging(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
namespace Microsoft.EntityFrameworkCore.Query;

public class NorthwindQueryInMemoryFixture<TModelCustomizer> : NorthwindQueryFixtureBase<TModelCustomizer>
where TModelCustomizer : IModelCustomizer, new()
where TModelCustomizer : ITestModelCustomizer, new()
{
protected override ITestStoreFactory TestStoreFactory
=> InMemoryTestStoreFactory.Instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Microsoft.EntityFrameworkCore.BulkUpdates;

public abstract class NorthwindBulkUpdatesFixture<TModelCustomizer> : NorthwindQueryRelationalFixture<TModelCustomizer>,
IBulkUpdatesFixtureBase
where TModelCustomizer : IModelCustomizer, new()
where TModelCustomizer : ITestModelCustomizer, new()
{
protected override string StoreName
=> "BulkUpdatesNorthwind";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
namespace Microsoft.EntityFrameworkCore.Query;

public abstract class NorthwindQueryRelationalFixture<TModelCustomizer> : NorthwindQueryFixtureBase<TModelCustomizer>
where TModelCustomizer : IModelCustomizer, new()
where TModelCustomizer : ITestModelCustomizer, new()
{
public new RelationalTestStore TestStore
=> (RelationalTestStore)base.TestStore;
Expand All @@ -14,8 +14,7 @@ public TestSqlLoggerFactory TestSqlLoggerFactory

public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder)
=> base.AddOptions(builder).ConfigureWarnings(
c => c
.Log(RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning))
c => c.Log(RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning))
.EnableDetailedErrors();

protected override bool ShouldLogCategory(string logCategory)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.EntityFrameworkCore.TestModels.ConcurrencyModel;
using Microsoft.EntityFrameworkCore.TestModels.Northwind;

// ReSharper disable AccessToDisposedClosure
Expand All @@ -9,7 +10,7 @@
namespace Microsoft.EntityFrameworkCore.Query;

public abstract class SqlExecutorTestBase<TFixture> : IClassFixture<TFixture>
where TFixture : NorthwindQueryRelationalFixture<NoopModelCustomizer>, new()
where TFixture : NorthwindQueryRelationalFixture<SqlExecutorModelCustomizer>, new()
{
protected SqlExecutorTestBase(TFixture fixture)
{
Expand Down Expand Up @@ -284,6 +285,25 @@ public virtual async Task Query_with_DbParameters_interpolated_2(bool async)
Assert.Equal(-1, actual);
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual async Task Query_with_parameters_custom_converter(bool async)
{
var city = new City { Name = "London" };
var contactTitle = "Sales Representative";

using var context = CreateContext();

var actual = async
? await context.Database.ExecuteSqlAsync(
$@"SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = {city} AND ""ContactTitle"" = {contactTitle}")
: context.Database.ExecuteSql(
$@"SELECT COUNT(*) FROM ""Customers"" WHERE ""City"" = {city} AND ""ContactTitle"" = {contactTitle}");

Assert.Equal(-1, actual);
}

private static Customer Process(Customer c, ManualResetEventSlim e, SemaphoreSlim s)
{
e.Set();
Expand All @@ -303,3 +323,19 @@ protected NorthwindContext CreateContext()

protected abstract string CustomerOrderHistoryWithGeneratedParameterSproc { get; }
}

public class SqlExecutorModelCustomizer : NoopModelCustomizer
{
public override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
{
configurationBuilder.DefaultTypeMapping<City>().HasConversion<CityToStringConverter>();
}

private sealed class CityToStringConverter : ValueConverter<City, string>
{
public CityToStringConverter()
: base(value => value.Name, value => new City { Name = value })
{
}
}
}
Loading

0 comments on commit af13549

Please sign in to comment.