diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs index d159e00482c..3c02c6739bd 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.RelationalProjectionBindingRemovingExpressionVisitor.cs @@ -7,6 +7,8 @@ using System.Linq; using System.Linq.Expressions; using System.Reflection; +using System.Runtime.CompilerServices; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Query.Internal; @@ -22,10 +24,15 @@ private sealed class RelationalProjectionBindingRemovingExpressionVisitor : Expr { private static readonly MethodInfo _isDbNullMethod = typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.IsDBNull), new[] { typeof(int) }); + private static readonly MethodInfo _getFieldValueMethod = + typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.GetFieldValue), new[] { typeof(int) }); + private static readonly MethodInfo _throwReadValueExceptionMethod = + typeof(RelationalProjectionBindingRemovingExpressionVisitor).GetTypeInfo().GetDeclaredMethod(nameof(ThrowReadValueException)); private readonly SelectExpression _selectExpression; private readonly ParameterExpression _dbDataReaderParameter; private readonly ParameterExpression _indexMapParameter; + private readonly bool _detailedErrorsEnabled; private readonly IDictionary> _materializationContextBindings = new Dictionary>(); @@ -34,11 +41,13 @@ public RelationalProjectionBindingRemovingExpressionVisitor( SelectExpression selectExpression, ParameterExpression dbDataReaderParameter, ParameterExpression indexMapParameter, + bool detailedErrorsEnabled, bool buffer) { _selectExpression = selectExpression; _dbDataReaderParameter = dbDataReaderParameter; _indexMapParameter = indexMapParameter; + _detailedErrorsEnabled = detailedErrorsEnabled; if (buffer) { ProjectionColumns = new ReaderColumn[selectExpression.Projection.Count]; @@ -108,7 +117,8 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp projectionIndex, IsNullableProjection(projection), property.GetRelationalTypeMapping(), - methodCallExpression.Type); + methodCallExpression.Type, + property); } return base.VisitMethodCall(methodCallExpression); @@ -149,7 +159,8 @@ private Expression CreateGetValueExpression( int index, bool nullable, RelationalTypeMapping typeMapping, - Type clrType) + Type clrType, + IPropertyBase property = null) { var getMethod = typeMapping.GetDataReaderMethod(); @@ -221,34 +232,27 @@ Expression valueExpression valueExpression = Expression.Convert(valueExpression, clrType); } - //var exceptionParameter - // = Expression.Parameter(typeof(Exception), name: "e"); - - //var property = materializationInfo.Property; - - //if (detailedErrorsEnabled) - //{ - // var catchBlock - // = Expression - // .Catch( - // exceptionParameter, - // Expression.Call( - // _throwReadValueExceptionMethod - // .MakeGenericMethod(valueExpression.Type), - // exceptionParameter, - // Expression.Call( - // dataReaderExpression, - // _getFieldValueMethod.MakeGenericMethod(typeof(object)), - // indexExpression), - // Expression.Constant(property, typeof(IPropertyBase)))); - - // valueExpression = Expression.TryCatch(valueExpression, catchBlock); - //} - - //if (box && valueExpression.Type.GetTypeInfo().IsValueType) - //{ - // valueExpression = Expression.Convert(valueExpression, typeof(object)); - //} + var exceptionParameter + = Expression.Parameter(typeof(Exception), name: "e"); + + if (_detailedErrorsEnabled) + { + var catchBlock + = Expression + .Catch( + exceptionParameter, + Expression.Call( + _throwReadValueExceptionMethod + .MakeGenericMethod(valueExpression.Type), + exceptionParameter, + Expression.Call( + dbDataReader, + _getFieldValueMethod.MakeGenericMethod(typeof(object)), + indexExpression), + Expression.Constant(property, typeof(IPropertyBase)))); + + valueExpression = Expression.TryCatch(valueExpression, catchBlock); + } if (nullable) { @@ -261,6 +265,44 @@ Expression valueExpression return valueExpression; } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static TValue ThrowReadValueException( + Exception exception, object value, IPropertyBase property = null) + { + var expectedType = typeof(TValue); + var actualType = value?.GetType(); + + string message; + + if (property != null) + { + var entityType = property.DeclaringType.DisplayName(); + var propertyName = property.Name; + if (expectedType == typeof(object)) + { + expectedType = property.ClrType; + } + + message = exception is NullReferenceException + || Equals(value, DBNull.Value) + ? CoreStrings.ErrorMaterializingPropertyNullReference(entityType, propertyName, expectedType) + : exception is InvalidCastException + ? CoreStrings.ErrorMaterializingPropertyInvalidCast(entityType, propertyName, expectedType, actualType) + : CoreStrings.ErrorMaterializingProperty(entityType, propertyName); + } + else + { + message = exception is NullReferenceException + || Equals(value, DBNull.Value) + ? CoreStrings.ErrorMaterializingValueNullReference(expectedType) + : exception is InvalidCastException + ? CoreStrings.ErrorMaterializingValueInvalidCast(expectedType, actualType) + : CoreStrings.ErrorMaterializingValue; + } + + throw new InvalidOperationException(message, exception); + } } } } diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs index e695596cf26..5cbc05c5115 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs @@ -21,6 +21,7 @@ public partial class RelationalShapedQueryCompilingExpressionVisitor : ShapedQue private readonly Type _contextType; private readonly IDiagnosticsLogger _logger; private readonly ISet _tags; + private readonly bool _detailedErrorsEnabled; private readonly bool _useRelationalNulls; public RelationalShapedQueryCompilingExpressionVisitor( @@ -36,6 +37,7 @@ public RelationalShapedQueryCompilingExpressionVisitor( _contextType = queryCompilationContext.ContextType; _logger = queryCompilationContext.Logger; _tags = queryCompilationContext.Tags; + _detailedErrorsEnabled = relationalDependencies.CoreSingletonOptions.AreDetailedErrorsEnabled; _useRelationalNulls = RelationalOptionsExtension.Extract(queryCompilationContext.ContextOptions).UseRelationalNulls; } @@ -66,6 +68,7 @@ protected override Expression VisitShapedQueryExpression(ShapedQueryExpression s selectExpression, dataReaderParameter, isNonComposedFromSql ? indexMapParameter : null, + _detailedErrorsEnabled, IsBuffering) .Visit(shaper, out var projectionColumns); diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitorDependencies.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitorDependencies.cs index 1ca17399f3a..f0a4191305f 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitorDependencies.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitorDependencies.cs @@ -58,7 +58,8 @@ public RelationalShapedQueryCompilingExpressionVisitorDependencies( [NotNull] IQuerySqlGeneratorFactory querySqlGeneratorFactory, [NotNull] ISqlExpressionFactory sqlExpressionFactory, [NotNull] IParameterNameGeneratorFactory parameterNameGeneratorFactory, - [NotNull] IRelationalParameterBasedQueryTranslationPostprocessorFactory relationalParameterBasedQueryTranslationPostprocessorFactory) + [NotNull] IRelationalParameterBasedQueryTranslationPostprocessorFactory relationalParameterBasedQueryTranslationPostprocessorFactory, + [NotNull] ICoreSingletonOptions coreSingletonOptions) { Check.NotNull(querySqlGeneratorFactory, nameof(querySqlGeneratorFactory)); Check.NotNull(sqlExpressionFactory, nameof(sqlExpressionFactory)); @@ -69,6 +70,7 @@ public RelationalShapedQueryCompilingExpressionVisitorDependencies( SqlExpressionFactory = sqlExpressionFactory; ParameterNameGeneratorFactory = parameterNameGeneratorFactory; RelationalParameterBasedQueryTranslationPostprocessorFactory = relationalParameterBasedQueryTranslationPostprocessorFactory; + CoreSingletonOptions = coreSingletonOptions; } /// @@ -91,6 +93,11 @@ public RelationalShapedQueryCompilingExpressionVisitorDependencies( /// public IRelationalParameterBasedQueryTranslationPostprocessorFactory RelationalParameterBasedQueryTranslationPostprocessorFactory { get; } + /// + /// Core singleton options. + /// + public ICoreSingletonOptions CoreSingletonOptions { get; } + /// /// Clones this dependency parameter object with one service replaced. /// @@ -102,7 +109,8 @@ public RelationalShapedQueryCompilingExpressionVisitorDependencies With( querySqlGeneratorFactory, SqlExpressionFactory, ParameterNameGeneratorFactory, - RelationalParameterBasedQueryTranslationPostprocessorFactory); + RelationalParameterBasedQueryTranslationPostprocessorFactory, + CoreSingletonOptions); /// /// Clones this dependency parameter object with one service replaced. @@ -114,7 +122,8 @@ public RelationalShapedQueryCompilingExpressionVisitorDependencies With([NotNull QuerySqlGeneratorFactory, sqlExpressionFactory, ParameterNameGeneratorFactory, - RelationalParameterBasedQueryTranslationPostprocessorFactory); + RelationalParameterBasedQueryTranslationPostprocessorFactory, + CoreSingletonOptions); /// /// Clones this dependency parameter object with one service replaced. @@ -127,7 +136,8 @@ public RelationalShapedQueryCompilingExpressionVisitorDependencies With( QuerySqlGeneratorFactory, SqlExpressionFactory, parameterNameGeneratorFactory, - RelationalParameterBasedQueryTranslationPostprocessorFactory); + RelationalParameterBasedQueryTranslationPostprocessorFactory, + CoreSingletonOptions); /// /// Clones this dependency parameter object with one service replaced. @@ -140,6 +150,21 @@ public RelationalShapedQueryCompilingExpressionVisitorDependencies With( QuerySqlGeneratorFactory, SqlExpressionFactory, ParameterNameGeneratorFactory, - relationalParameterBasedQueryTranslationPostprocessorFactory); + relationalParameterBasedQueryTranslationPostprocessorFactory, + CoreSingletonOptions); + + /// + /// Clones this dependency parameter object with one service replaced. + /// + /// A replacement for the current dependency of this type. + /// A new parameter object with the given service replaced. + public RelationalShapedQueryCompilingExpressionVisitorDependencies With( + [NotNull] ICoreSingletonOptions coreSingletonOptions) + => new RelationalShapedQueryCompilingExpressionVisitorDependencies( + QuerySqlGeneratorFactory, + SqlExpressionFactory, + ParameterNameGeneratorFactory, + RelationalParameterBasedQueryTranslationPostprocessorFactory, + coreSingletonOptions); } } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index c727535c53c..2caa9a31df1 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -108,7 +108,9 @@ public bool IsNonComposedFromSql() && Tables[0] is FromSqlExpression fromSql && Projection.All( pe => pe.Expression is ColumnExpression column - && string.Equals(fromSql.Alias, column.Table.Alias, StringComparison.OrdinalIgnoreCase)); + && string.Equals(fromSql.Alias, column.Table.Alias, StringComparison.OrdinalIgnoreCase)) + && _projectionMapping.TryGetValue(new ProjectionMember(), out var mapping) + && mapping.Type == typeof(Dictionary); public void ApplyProjection() { diff --git a/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs index 85c76e27324..6ce0743894b 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs @@ -32,7 +32,7 @@ protected FromSqlQueryTestBase(TFixture fixture) protected TFixture Fixture { get; } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void Bad_data_error_handling_invalid_cast_key() { using var context = CreateContext(); @@ -47,7 +47,7 @@ public virtual void Bad_data_error_handling_invalid_cast_key() .ToList()).Message); } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void Bad_data_error_handling_invalid_cast() { using var context = CreateContext(); @@ -62,7 +62,7 @@ public virtual void Bad_data_error_handling_invalid_cast() .ToList()).Message); } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void Bad_data_error_handling_invalid_cast_projection() { using var context = CreateContext(); @@ -78,7 +78,7 @@ public virtual void Bad_data_error_handling_invalid_cast_projection() .ToList()).Message); } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void Bad_data_error_handling_invalid_cast_no_tracking() { using var context = CreateContext(); @@ -94,15 +94,10 @@ public virtual void Bad_data_error_handling_invalid_cast_no_tracking() .ToList()).Message); } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void Bad_data_error_handling_null() { using var context = CreateContext(); - context.Set().FromSqlRaw( - NormalizeDelimitersInRawString( - @"SELECT [ProductID], [ProductName], [SupplierID], [UnitPrice], [UnitsInStock], NULL AS [Discontinued] - FROM [Products]")) - .ToList(); Assert.Equal( CoreStrings.ErrorMaterializingPropertyNullReference("Product", "Discontinued", typeof(bool)), Assert.Throws( @@ -114,12 +109,12 @@ public virtual void Bad_data_error_handling_null() .ToList()).Message); } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void Bad_data_error_handling_null_projection() { using var context = CreateContext(); Assert.Equal( - CoreStrings.ErrorMaterializingPropertyNullReference("Product", "Discontinued", typeof(bool)), + CoreStrings.ErrorMaterializingValueNullReference(typeof(bool)), Assert.Throws( () => context.Set().FromSqlRaw( @@ -130,7 +125,7 @@ public virtual void Bad_data_error_handling_null_projection() .ToList()).Message); } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void Bad_data_error_handling_null_no_tracking() { using var context = CreateContext(); @@ -184,7 +179,7 @@ public virtual void FromSqlRaw_queryable_simple_columns_out_of_order_and_extra_c Assert.Equal(91, context.ChangeTracker.Entries().Count()); } - [ConditionalFact(Skip = "#15751")] + [ConditionalFact] public virtual void FromSqlRaw_queryable_simple_columns_out_of_order_and_not_enough_columns_throws() { using var context = CreateContext(); diff --git a/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs index 33ed6b13d65..98cfe0ffff4 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/FromSqlSprocQueryTestBase.cs @@ -75,12 +75,11 @@ public virtual async Task From_sql_queryable_stored_procedure_projection(bool as .FromSqlRaw(TenMostExpensiveProductsSproc, GetTenMostExpensiveProductsParameters()) .Select(mep => mep.TenMostExpensiveProducts); - var actual = async - ? await query.ToArrayAsync() - : query.ToArray(); - - Assert.Equal(10, actual.Length); - Assert.Contains(actual, r => r == "Côte de Blaye"); + Assert.Equal( + RelationalStrings.FromSqlNonComposable, + (async + ? await Assert.ThrowsAsync(() => query.ToArrayAsync()) + : Assert.Throws(() => query.ToArray())).Message); } [ConditionalTheory] diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs index db766652466..4c25fa22175 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlQuerySqlServerTest.cs @@ -396,15 +396,14 @@ public override void FromSqlRaw_queryable_simple_projection_composed() { base.FromSqlRaw_queryable_simple_projection_composed(); - // issue #16079 - // AssertSql( - // @"SELECT [p].[ProductName] - //FROM ( - // SELECT * - // FROM ""Products"" - // WHERE ""Discontinued"" <> CAST(1 AS bit) - // AND ((""UnitsInStock"" + ""UnitsOnOrder"") < ""ReorderLevel"") - //) AS [p]"); + AssertSql( + @"SELECT [p].[ProductName] +FROM ( + SELECT * + FROM ""Products"" + WHERE ""Discontinued"" <> CAST(1 AS bit) + AND ((""UnitsInStock"" + ""UnitsOnOrder"") < ""ReorderLevel"") +) AS [p]"); } public override void FromSqlRaw_queryable_simple_include() @@ -717,7 +716,10 @@ public override void FromSqlInterpolated_parameterization_issue_12213() AssertSql( @"p0='10300' -SELECT * FROM ""Orders"" WHERE ""OrderID"" >= @p0", +SELECT [o].[OrderID] +FROM ( + SELECT * FROM ""Orders"" WHERE ""OrderID"" >= @p0 +) AS [o]", // @"@__max_0='10400' p0='10300' @@ -783,6 +785,54 @@ UNION ALL ) AS [c0]"); } + [ConditionalFact(Skip = "Issue#20364")] + public override void Bad_data_error_handling_invalid_cast() + { + base.Bad_data_error_handling_invalid_cast(); + } + + [ConditionalFact(Skip = "Issue#20364")] + public override void Bad_data_error_handling_invalid_cast_key() + { + base.Bad_data_error_handling_invalid_cast_key(); + } + + [ConditionalFact(Skip = "Issue#20364")] + public override void Bad_data_error_handling_invalid_cast_no_tracking() + { + base.Bad_data_error_handling_invalid_cast_no_tracking(); + } + + [ConditionalFact(Skip = "Issue#20364")] + public override void Bad_data_error_handling_invalid_cast_projection() + { + base.Bad_data_error_handling_invalid_cast_projection(); + } + + [ConditionalFact(Skip = "Issue#20364")] + public override void Bad_data_error_handling_null() + { + base.Bad_data_error_handling_null(); + } + + [ConditionalFact(Skip = "Issue#20364")] + public override void Bad_data_error_handling_null_no_tracking() + { + base.Bad_data_error_handling_null_no_tracking(); + } + + [ConditionalFact(Skip = "Issue#20364")] + public override void Bad_data_error_handling_null_projection() + { + base.Bad_data_error_handling_null_projection(); + } + + [ConditionalFact(Skip = "Issue#20364")] + public override void FromSqlRaw_queryable_simple_columns_out_of_order_and_not_enough_columns_throws() + { + base.FromSqlRaw_queryable_simple_columns_out_of_order_and_not_enough_columns_throws(); + } + protected override DbParameter CreateDbParameter(string name, object value) => new SqlParameter { ParameterName = name, Value = value }; diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs index fd0f53fa119..311e52dc1e0 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/FromSqlSprocQuerySqlServerTest.cs @@ -31,13 +31,6 @@ public override async Task From_sql_queryable_stored_procedure_with_tag(bool asy [dbo].[Ten Most Expensive Products]"); } - public override async Task From_sql_queryable_stored_procedure_projection(bool async) - { - await base.From_sql_queryable_stored_procedure_projection(async); - - AssertSql("[dbo].[Ten Most Expensive Products]"); - } - public override async Task From_sql_queryable_stored_procedure_with_parameter(bool async) { await base.From_sql_queryable_stored_procedure_with_parameter(async); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index 8bb83b5b233..bd9249a09c8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs @@ -442,7 +442,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) #endregion - [ConditionalFact(Skip = "Issue#15751")] + [ConditionalFact(Skip = "Issue#20364")] public void Query_when_null_key_in_database_should_throw() { using var testStore = SqlServerTestStore.CreateInitialized("QueryBugsTest"); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs index 259ddbadfe1..4891d8b4702 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/BadDataSqliteTest.cs @@ -20,10 +20,7 @@ // ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore.Query { - // Issue #15751 -#pragma warning disable xUnit1000 // Test classes must be public - internal class BadDataSqliteTest : IClassFixture -#pragma warning restore xUnit1000 // Test classes must be public + public class BadDataSqliteTest : IClassFixture { public BadDataSqliteTest(BadDataSqliteFixture fixture) => Fixture = fixture; @@ -67,7 +64,7 @@ public void Bad_data_error_handling_invalid_cast_projection() { using var context = CreateContext(1); Assert.Equal( - CoreStrings.ErrorMaterializingPropertyInvalidCast("Product", "ProductName", typeof(string), typeof(int)), + CoreStrings.ErrorMaterializingValueInvalidCast(typeof(string), typeof(int)), Assert.Throws( () => context.Set().Where(p => p.ProductID != 4) @@ -105,7 +102,7 @@ public void Bad_data_error_handling_null_projection() { using var context = CreateContext(new object[] { null }); Assert.Equal( - CoreStrings.ErrorMaterializingPropertyNullReference("Product", "Discontinued", typeof(bool)), + CoreStrings.ErrorMaterializingValueNullReference(typeof(bool)), Assert.Throws( () => context.Set()