Skip to content

Commit

Permalink
Query: Properly generate expression to read from BufferedDataReader (#…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@smitpatel
smitpatel committed Nov 17, 2020
1 parent d1051ea commit d185ae4
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 22 deletions.
88 changes: 66 additions & 22 deletions ...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down
89 changes: 89 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Location23282> 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<ILoggerFactory>(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<TContext>(
Expand Down

0 comments on commit d185ae4

Please sign in to comment.