From d185ae41475c7c8cbf152e50d3bd9ddb3e9c17e2 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 17 Nov 2020 15:24:20 -0800 Subject: [PATCH] Query: Properly generate expression to read from BufferedDataReader (#23295) Resolves #23282 Resolves #23104 Issue: When buffering is enabled, first time when we read particular property slot, we create expression to read into buffer and out of the buffer. We mistakenly bypassed changing expression to read from value buffer when same property slot was being read again, causing cast errors Also matches the type being read out of the buffered data reader --- ...sitor.ShaperProcessingExpressionVisitor.cs | 88 +++++++++++++----- .../Query/QueryBugsTest.cs | 89 +++++++++++++++++++ 2 files changed, 155 insertions(+), 22 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs index cce1f452601..1e88389177e 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs @@ -994,33 +994,77 @@ Expression valueExpression indexExpression); var buffering = false; - if (_readerColumns != null - && _readerColumns[index] == null) + + if (AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23282", out var isEnabled) && isEnabled) { - buffering = true; - var bufferedReaderLambdaExpression = valueExpression; - var columnType = bufferedReaderLambdaExpression.Type; - if (!columnType.IsValueType - || !BufferedDataReader.IsSupportedValueType(columnType)) + if (_readerColumns != null + && _readerColumns[index] == null) { - columnType = typeof(object); - bufferedReaderLambdaExpression = Expression.Convert(bufferedReaderLambdaExpression, columnType); - } + buffering = true; + var bufferedReaderLambdaExpression = valueExpression; + var columnType = bufferedReaderLambdaExpression.Type; + if (!columnType.IsValueType + || !BufferedDataReader.IsSupportedValueType(columnType)) + { + columnType = typeof(object); + bufferedReaderLambdaExpression = Expression.Convert(bufferedReaderLambdaExpression, columnType); + } - _readerColumns[index] = ReaderColumn.Create( - columnType, - nullable, - _indexMapParameter != null ? ((ColumnExpression)_selectExpression.Projection[index].Expression).Name : null, - property, - Expression.Lambda( - bufferedReaderLambdaExpression, - dbDataReader, - _indexMapParameter ?? Expression.Parameter(typeof(int[]))).Compile()); - - if (getMethod.DeclaringType != typeof(DbDataReader)) + _readerColumns[index] = ReaderColumn.Create( + columnType, + nullable, + _indexMapParameter != null ? ((ColumnExpression)_selectExpression.Projection[index].Expression).Name : null, + property, + Expression.Lambda( + bufferedReaderLambdaExpression, + dbDataReader, + _indexMapParameter ?? Expression.Parameter(typeof(int[]))).Compile()); + + if (getMethod.DeclaringType != typeof(DbDataReader)) + { + valueExpression = Expression.Call( + dbDataReader, RelationalTypeMapping.GetDataReaderMethod(columnType), indexExpression); + } + } + } + else + { + if (_readerColumns != null) { + buffering = true; + var columnType = valueExpression.Type; + var bufferedColumnType = columnType; + if (!bufferedColumnType.IsValueType + || !BufferedDataReader.IsSupportedValueType(bufferedColumnType)) + { + bufferedColumnType = typeof(object); + } + + if (_readerColumns[index] == null) + { + var bufferedReaderLambdaExpression = valueExpression; + if (columnType != bufferedColumnType) + { + bufferedReaderLambdaExpression = Expression.Convert(bufferedReaderLambdaExpression, bufferedColumnType); + } + + _readerColumns[index] = ReaderColumn.Create( + bufferedColumnType, + nullable, + _indexMapParameter != null ? ((ColumnExpression)_selectExpression.Projection[index].Expression).Name : null, + property, + Expression.Lambda( + bufferedReaderLambdaExpression, + dbDataReader, + _indexMapParameter ?? Expression.Parameter(typeof(int[]))).Compile()); + } + valueExpression = Expression.Call( - dbDataReader, RelationalTypeMapping.GetDataReaderMethod(columnType), indexExpression); + dbDataReader, RelationalTypeMapping.GetDataReaderMethod(bufferedColumnType), indexExpression); + if (valueExpression.Type != columnType) + { + valueExpression = Expression.Convert(valueExpression, columnType); + } } } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs index 9ff61110109..56f76a7b1f1 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs @@ -23,6 +23,7 @@ using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using NetTopologySuite.Geometries; using Xunit; using Xunit.Abstractions; @@ -9290,6 +9291,94 @@ private SqlServerTestStore CreateDatabase23211() #endregion + #region Issue23282 + + [ConditionalFact] + public virtual void Can_query_point_with_buffered_data_reader() + { + var (options, testSqlLoggerFactory) = CreateOptions23282(); + using var context = new MyContext23282(options); + + var testUser = context.Locations.FirstOrDefault(x => x.Name == "My Location"); + + Assert.NotNull(testUser); + + testSqlLoggerFactory.AssertBaseline( + new[] { + @"SELECT TOP(1) [l].[Id], [l].[Name], [l].[Address_County], [l].[Address_Line1], [l].[Address_Line2], [l].[Address_Point], [l].[Address_Postcode], [l].[Address_Town] +FROM [Locations] AS [l] +WHERE [l].[Name] = N'My Location'" }); + } + + [Owned] + private class Address23282 + { + public string Line1 { get; set; } + public string Line2 { get; set; } + public string Town { get; set; } + public string County { get; set; } + public string Postcode { get; set; } + + public Point Point { get; set; } + } + + private class Location23282 + { + [Key] + [DatabaseGenerated(DatabaseGeneratedOption.Identity)] + public Guid Id { get; set; } + + public string Name { get; set; } + public Address23282 Address { get; set; } + } + + private class MyContext23282 : DbContext + { + public DbSet Locations { get; set; } + + public MyContext23282(DbContextOptions options) + : base(options) + { + } + } + + private (DbContextOptions, TestSqlLoggerFactory) CreateOptions23282() + { + var testStore = SqlServerTestStore.CreateInitialized("QueryBugsTest"); + var testSqlLoggerFactory = new TestSqlLoggerFactory(); + var serviceProvider = new ServiceCollection().AddSingleton(testSqlLoggerFactory).BuildServiceProvider(); + + var optionsBuilder = Fixture.AddOptions(new DbContextOptionsBuilder() + .UseSqlServer(testStore.ConnectionString, b => b.EnableRetryOnFailure().UseNetTopologySuite())) + .EnableDetailedErrors() + .EnableServiceProviderCaching(false) + .UseApplicationServiceProvider(serviceProvider); + + var context = new MyContext23282(optionsBuilder.Options); + if (context.Database.EnsureCreatedResiliently()) + { + context.Locations.Add(new Location23282 + { + Name = "My Location", + Address = new Address23282 + { + Line1 = "1 Fake Street", + Town = "Fake Town", + County = "Fakeshire", + Postcode = "PO57 0DE", + Point = new Point(115.7930, 37.2431) { SRID = 4326 } + } + }); + context.SaveChanges(); + } + + testSqlLoggerFactory.Clear(); + + return (optionsBuilder.Options, testSqlLoggerFactory); + } + + #endregion + private DbContextOptions _options; private SqlServerTestStore CreateTestStore(