Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect NullReferenceException for parameter in split query with GroupBy #30022

Closed
roji opened this issue Jan 10, 2023 · 0 comments · Fixed by #30024
Closed

Incorrect NullReferenceException for parameter in split query with GroupBy #30022

roji opened this issue Jan 10, 2023 · 0 comments · Fixed by #30024
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Milestone

Comments

@roji
Copy link
Member

roji commented Jan 10, 2023

Reported by @excelkobayashi in #28940 (comment), with the following repro (thanks!):

Program.cs:

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;

namespace QueryTest;

internal static class Program
{
	private static async Task Main()
	{
		using ServiceProvider services = Setup.CreateServices();

		using(IServiceScope seedScope = services.CreateScope())
		{
			MyContext context = seedScope.ServiceProvider.GetRequiredService<MyContext>();
			await Setup.Seed(context);
		}

		string userId = "1";
		string[] valueIds = new[] { "A", "B" };

		using(IServiceScope scope1 = services.CreateScope())
		{
			MyContext context = scope1.ServiceProvider.GetRequiredService<MyContext>();
			_ = await context.MyData.FindOtherUserValues1(userId, valueIds);
		}

		using(IServiceScope scope2 = services.CreateScope())
		{
			MyContext context = scope2.ServiceProvider.GetRequiredService<MyContext>();
			_ = await context.MyData.FindOtherUserValues2(userId, valueIds);
		}
	}
}

internal record MyData(string UserId, string? ValueId, int Id = 0) { }

internal static class Setup
{
	private static readonly IReadOnlyList<string> _seedUserIds = new string[] { "1", "2", "3" };
	private static readonly IReadOnlyList<string?> _seedValueIds = new string?[] { "A", "B", "C", null };

	public static ServiceProvider CreateServices()
	{
		return new ServiceCollection()
			.AddDbContext<MyContext>()
			.BuildServiceProvider(true);
	}

	public static async Task Seed(MyContext context)
	{
		_ = await context.Database.EnsureDeletedAsync();
		_ = await context.Database.EnsureCreatedAsync();

		foreach(string userId in _seedUserIds)
		{
			foreach(string? valueId in _seedValueIds)
			{
				MyData data = new(userId, valueId);
				_ = context.MyData.Add(data);
			}
		}

		_ = await context.SaveChangesAsync();
	}
}

internal class MyContext : DbContext
{
	required public DbSet<MyData> MyData { get; init; }

	protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
	{
		_ = optionsBuilder.UseSqlServer(@$"Server=(localdb)\mssqllocaldb;Database={nameof(QueryTest)};ConnectRetryCount=0",
			options => options.UseQuerySplittingBehavior(QuerySplittingBehavior.SplitQuery));
	}
}

internal static class Queries
{
	public static async Task<IReadOnlyDictionary<string, IReadOnlyList<string>>> FindOtherUserValues1(this IQueryable<MyData> source, string? excludeUserId, IEnumerable<string> includeValueIds, CancellationToken cancellationToken = default)
	{
		IQueryable<MyData> dataQuery = source
			.AsNoTracking()
			.Where(md => md.UserId != excludeUserId)
			.Where(md => includeValueIds.Contains(md.ValueId!));

		List<MyData> matches = await dataQuery.ToListAsync(cancellationToken);

		var groupOnClient = matches
			.GroupBy(md => md.ValueId!)
			.Select(grp => new { ValueId = grp.Key, UserIds = grp.Select(md => md.UserId!) });

		// No crash
		return groupOnClient.ToDictionary(grp => grp.ValueId, grp => (IReadOnlyList<string>)grp.UserIds.ToList());
	}

	public static async Task<IReadOnlyDictionary<string, IReadOnlyList<string>>> FindOtherUserValues2(this IQueryable<MyData> source, string? excludeUserId, IEnumerable<string> includeValueIds, CancellationToken cancellationToken = default)
	{
		IQueryable<MyData> dataQuery = source
			.AsNoTracking()
			.Where(md => md.UserId != excludeUserId)
			.Where(md => includeValueIds.Contains(md.ValueId!));

		var groupOnServer = dataQuery
			.GroupBy(md => md.ValueId!)
			.Select(grp => new { ValueId = grp.Key, UserIds = grp.Select(md => md.UserId!) });

		// Crashes here
		var groups = await groupOnServer.ToListAsync(cancellationToken);

		return groups.ToDictionary(grp => grp.ValueId, grp => (IReadOnlyList<string>)grp.UserIds.ToList());
	}
}

QueryTest.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="7.0.2" />
  </ItemGroup>

</Project>
@roji roji self-assigned this Jan 10, 2023
@roji roji changed the title Incorrect NullReferenceException in split query with GroupBy Incorrect NullReferenceException for parameter in split query with GroupBy Jan 10, 2023
roji added a commit to roji/efcore that referenced this issue Jan 11, 2023
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 11, 2023
roji added a commit to roji/efcore that referenced this issue Jan 11, 2023
@ghost ghost closed this as completed in #30024 Jan 11, 2023
ghost pushed a commit that referenced this issue Jan 11, 2023
roji added a commit to roji/efcore that referenced this issue Jan 11, 2023
roji added a commit to roji/efcore that referenced this issue Jan 11, 2023
@roji roji reopened this Jan 11, 2023
@ajcvickers ajcvickers added this to the 7.0.x milestone Jan 12, 2023
@ajcvickers ajcvickers modified the milestones: 7.0.x, 7.0.3 Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants