From 2033f65e215c4c45589d827ec6578550964baf27 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 12 Nov 2020 13:53:42 -0800 Subject: [PATCH] Query: Properly generate expression to read from BufferedDataReader 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 5dcee3c6f7d..c2ce25d70f6 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; @@ -9139,6 +9140,94 @@ private SqlServerTestStore CreateDatabase22568() #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(