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

GroupBy debug assert: Missing alias in the list: p1,p #26104

Closed
Tracked by #30173
ajcvickers opened this issue Sep 19, 2021 · 6 comments · Fixed by #27011 or #32785
Closed
Tracked by #30173

GroupBy debug assert: Missing alias in the list: p1,p #26104

ajcvickers opened this issue Sep 19, 2021 · 6 comments · Fixed by #27011 or #32785
Labels
area-groupby area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Milestone

Comments

@ajcvickers
Copy link
Contributor

Found working on samples for RC1, then running them as tests on main.

System.InvalidOperationException
Missing alias in the list: p1,p
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryTranslationPostprocessor.TableAliasVerifyingExpressionVisitor.ScopedVisitor.EntryPoint(Expression expression) in C:\dotnet\efcore\src\EFCore.Relational\Query\RelationalQueryTranslationPostprocessor.cs:line 128
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryTranslationPostprocessor.TableAliasVerifyingExpressionVisitor.UniquifyAliasInSelectExpression(Expression selectExpression) in C:\dotnet\efcore\src\EFCore.Relational\Query\RelationalQueryTranslationPostprocessor.cs:line 104
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryTranslationPostprocessor.TableAliasVerifyingExpressionVisitor.Visit(Expression expression) in C:\dotnet\efcore\src\EFCore.Relational\Query\RelationalQueryTranslationPostprocessor.cs:line 89
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryTranslationPostprocessor.Process(Expression query) in C:\dotnet\efcore\src\EFCore.Relational\Query\RelationalQueryTranslationPostprocessor.cs:line 52
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query) in C:\dotnet\efcore\src\EFCore\Query\QueryCompilationContext.cs:line 190
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async) in C:\dotnet\efcore\src\EFCore\Storage\Database.cs:line 76
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async) in C:\dotnet\efcore\src\EFCore\Query\Internal\QueryCompiler.cs:line 111
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0() in C:\dotnet\efcore\src\EFCore\Query\Internal\QueryCompiler.cs:line 95
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler) in C:\dotnet\efcore\src\EFCore\Query\Internal\CompiledQueryCache.cs:line 74
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query) in C:\dotnet\efcore\src\EFCore\Query\Internal\QueryCompiler.cs:line 91
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression) in C:\dotnet\efcore\src\EFCore\Query\Internal\EntityQueryProvider.cs:line 78
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator() in C:\dotnet\efcore\src\EFCore\Query\Internal\EntityQueryable`.cs:line 90
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at GroupBySample.Translate_GroupBy_followed_by_FirstOrDefault_over_group() in C:\dotnet\efcore\test\EFCore.SqlServer.FunctionalTests\SqlServerEndToEndTest.cs:line 31
public class GroupBySample
{
    [ConditionalFact]
    public void Translate_GroupBy_followed_by_FirstOrDefault_over_group()
    {
        Console.WriteLine($">>>> Sample: {nameof(Translate_GroupBy_followed_by_FirstOrDefault_over_group)}");
        Console.WriteLine();

        Helpers.RecreateCleanDatabase();
        Helpers.PopulateDatabase();

        // Example 3. From #12640
        using (var context = new ShoesContext())
        {
            #region GroupBy3
            var people = context.People
                .Where(e => e.MiddleInitial == "Q" && e.Age == 20)
                .GroupBy(e => e.LastName)
                .Select(g => g.First().LastName)
                .OrderBy(e => e.Length)
                .ToList();
            #endregion
        
            Console.WriteLine();
        
            foreach (var person in people)
            {
                Console.WriteLine(person);
            }
        
            Console.WriteLine();
        }
        
        // Example 5. From #12601
        using (var context = new ShoesContext())
        {
            #region GroupBy5
            var results = context.People
                .GroupBy(e => e.FirstName)
                .Select(g => g.First().LastName)
                .OrderBy(e => e)
                .ToList();
            #endregion
        
            Console.WriteLine();
            
            foreach (var result in results)
            {
                Console.WriteLine(result);
            }
        
            Console.WriteLine();
        }

        // Example 6. From #12600
        using (var context = new ShoesContext())
        {
            #region GrouoBy6
            var results = context.People.Where(e => e.Age == 20)
                .GroupBy(e => e.Id)
                .Select(g => g.First().MiddleInitial)
                .OrderBy(e => e)
                .ToList();
            #endregion
        
            Console.WriteLine();
        
            foreach (var result in results)
            {
                Console.WriteLine(result);
            }
        
            Console.WriteLine();
        }
    }

    public static class Helpers
    {
        public static void RecreateCleanDatabase()
        {
            using var context = new ShoesContext(quiet: true);

            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();
        }

        public static void PopulateDatabase()
        {
            using var context = new ShoesContext(quiet: true);

            context.AddRange(
                new Person
                {
                    FirstName = "Jim",
                    MiddleInitial = "A",
                    LastName = "Bob",
                    Age = 20,
                    Feet = new Feet { Size = 11 },
                    Shoes = { new() { Style = "Sneakers", Age = 19 }, new() { Style = "Dress", Age = 20 } }
                },
                new Person
                {
                    FirstName = "Tom",
                    MiddleInitial = "A",
                    LastName = "Bob",
                    Age = 20,
                    Feet = new Feet { Size = 12 },
                    Shoes = { new() { Style = "Sneakers", Age = 21 }, new() { Style = "Dress", Age = 19 } }
                },
                new Person
                {
                    FirstName = "Ben",
                    MiddleInitial = "Q",
                    LastName = "Bob",
                    Age = 20,
                    Feet = new Feet { Size = 12 },
                    Shoes = { new() { Style = "Sneakers", Age = 20 }, new() { Style = "Dress", Age = 21 } }
                },
                new Person
                {
                    FirstName = "Jim",
                    MiddleInitial = "Q",
                    LastName = "Jon",
                    Age = 20,
                    Feet = new Feet { Size = 11 },
                    Shoes = { new() { Style = "Sneakers", Age = 19 }, new() { Style = "Dress", Age = 20 } }
                },
                new Person
                {
                    FirstName = "Tom",
                    MiddleInitial = "A",
                    LastName = "Jon",
                    Age = 21,
                    Feet = new Feet { Size = 11 },
                    Shoes = { new() { Style = "Sneakers", Age = 21 }, new() { Style = "Dress", Age = 19 } }
                },
                new Person
                {
                    FirstName = "Ben",
                    MiddleInitial = "A",
                    LastName = "Jon",
                    Age = 21,
                    Feet = new Feet { Size = 12 },
                    Shoes = { new() { Style = "Sneakers", Age = 20 }, new() { Style = "Dress", Age = 21 } }
                },
                new Person
                {
                    FirstName = "Jim",
                    MiddleInitial = "Q",
                    LastName = "Don",
                    Age = 21,
                    Feet = new Feet { Size = 12 },
                    Shoes = { new() { Style = "Sneakers", Age = 19 }, new() { Style = "Dress", Age = 20 } }
                },
                new Person
                {
                    FirstName = "Tom",
                    MiddleInitial = "Q",
                    LastName = "Don",
                    Age = 21,
                    Feet = new Feet { Size = 11 },
                    Shoes = { new() { Style = "Sneakers", Age = 21 }, new() { Style = "Dress", Age = 19 } }
                },
                new Person
                {
                    FirstName = "Ben",
                    MiddleInitial = "A",
                    LastName = "Don",
                    Age = 21,
                    Feet = new Feet { Size = 11 },
                    Shoes = { new() { Style = "Sneakers", Age = 20 }, new() { Style = "Dress", Age = 21 } }
                },
                new Person
                {
                    FirstName = "Jim",
                    MiddleInitial = "A",
                    LastName = "Zee",
                    Age = 21,
                    Feet = new Feet { Size = 12 },
                    Shoes = { new() { Style = "Sneakers", Age = 19 }, new() { Style = "Dress", Age = 20 } }
                },
                new Person
                {
                    FirstName = "Tom",
                    MiddleInitial = "Q",
                    LastName = "Zee",
                    Age = 21,
                    Feet = new Feet { Size = 12 },
                    Shoes = { new() { Style = "Sneakers", Age = 21 }, new() { Style = "Dress", Age = 19 } }
                },
                new Person
                {
                    FirstName = "Ben",
                    MiddleInitial = "Q",
                    LastName = "Zee",
                    Age = 21,
                    Feet = new Feet { Size = 11 },
                    Shoes = { new() { Style = "Sneakers", Age = 20 }, new() { Style = "Dress", Age = 21 } }
                });

            context.SaveChanges();
        }
    }

    #region Model
    public class Person
    {
        public int Id { get; set; }
        public int Age { get; set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public string MiddleInitial { get; set; }
        public Feet Feet { get; set; }
        public ICollection<Shoes> Shoes { get; } = new List<Shoes>();
    }

    public class Shoes
    {
        public int Id { get; set; }
        public int Age { get; set; }
        public string Style { get; set; }
        public Person Person { get; set; }
    }

    public class Feet
    {
        public int Id { get; set; }
        public int Size { get; set; }
        public Person Person { get; set; }
    }
    #endregion

    public class ShoesContext : DbContext
    {
        public DbSet<Person> People { get; set; }
        public DbSet<Shoes> Shoes { get; set; }
        public DbSet<Feet> Feet { get; set; }

        private readonly bool _quiet;

        public ShoesContext(bool quiet = false)
        {
            _quiet = quiet;
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
                .EnableSensitiveDataLogging()
                .UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=EFCoreSample");

            if (!_quiet)
            {
                optionsBuilder.LogTo(Console.WriteLine, new[] { RelationalEventId.CommandExecuted });
            }
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Feet>().HasOne(e => e.Person).WithOne(e => e.Feet).HasForeignKey<Feet>();
        }
    }
}
@ajcvickers
Copy link
Contributor Author

@smitpatel Let's try to fix ASAP for 6.0.

@smitpatel
Copy link
Contributor

It might not be a regression. The check throwing exception only runs in debug build. If you have generated query from either version, please post.

@ajcvickers
Copy link
Contributor Author

@smitpatel All the SQL is in the docs PR which is using RC1: dotnet/EntityFramework.Docs#3436

@smitpatel
Copy link
Contributor

.Select(g => g.First().LastName)
                .OrderBy(e => e.Length)

is the core issue.
The selector needs to be something which is a scalar single result value so it gets lifted in projection and ordering is applied afterwards which copies earlier select and they both ends up getting shared so earlier alias is gone from the tree.
Overlaps with #20291 #7776 #18775
Since this is debug only check, we can differ to next release.

@smitpatel smitpatel removed this from the 6.0.0 milestone Sep 27, 2021
@ajcvickers ajcvickers changed the title GroupBy regression from RC1 to Main (and likely RC2) GroupBy debug assert: Missing alias in the list: p1,p Sep 28, 2021
@ajcvickers ajcvickers added this to the Backlog milestone Sep 29, 2021
@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Nov 10, 2021
smitpatel added a commit that referenced this issue Dec 15, 2021
…pplied

- Add an internal state to remember the mutability which is set to false after ApplyProjection is called.
- Add overload of ApplyProjection which ignores shaper to create an immutable select expression to be used as subquery
- Add debug check to verify that all SelectExpression are marked as immutable
- Clone mapped projection of SqlExpression kind before translating it further to avoid reference sharing with projection which may be applied at later stage

Resolves #26532 - all those expressions have debug check in ctor
Resolves #26167 - by having more accurate flag for immutability
Resolves #26104 - by cloning the projection before translating further. This issue may get resolved in other way after pending selector work if we reuse projections from subquery but regardless the change seems better to avoid reference sharing of SelectExpression objects
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 15, 2021
smitpatel added a commit that referenced this issue Dec 15, 2021
…pplied

- Add an internal state to remember the mutability which is set to false after ApplyProjection is called.
- Add overload of ApplyProjection which ignores shaper to create an immutable select expression to be used as subquery
- Add debug check to verify that all SelectExpression are marked as immutable
- Clone mapped projection of SqlExpression kind before translating it further to avoid reference sharing with projection which may be applied at later stage

Resolves #26532 - all those expressions have debug check in ctor
Resolves #26167 - by having more accurate flag for immutability
Resolves #26104 - by cloning the projection before translating further. This issue may get resolved in other way after pending selector work if we reuse projections from subquery but regardless the change seems better to avoid reference sharing of SelectExpression objects
@smitpatel smitpatel removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 15, 2021
@smitpatel
Copy link
Contributor

Attempted to fix this by cloning SqlExpression when getting it through projection during Sql translation (so that composed fragment differs from projection causing them not to overwrite the aliases), but that cause issues because we need to identify both of them as same when pushing down subquery so that aggregate operation is lifted inside group by rather than as a subquery outside the group by query. Since it makes SQL worse, this approach may not be right choice.

Re-visit with pending selector work.

@ajcvickers ajcvickers removed this from the 7.0.0 milestone Feb 14, 2022
@ajcvickers ajcvickers reopened this Feb 14, 2022
@AndriySvyryd AndriySvyryd added this to the 7.0.0 milestone Feb 15, 2022
@ajcvickers ajcvickers added punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. and removed propose-punt labels Jul 7, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, Backlog Jul 7, 2022
@smitpatel smitpatel removed their assignment Sep 14, 2022
@roji
Copy link
Member

roji commented Nov 29, 2023

Investigated this and root-caused it in #32455.

roji added a commit to roji/efcore that referenced this issue Jan 11, 2024
roji added a commit to roji/efcore that referenced this issue Jan 11, 2024
roji added a commit to roji/efcore that referenced this issue Jan 11, 2024
roji added a commit to roji/efcore that referenced this issue Jan 15, 2024
@roji roji closed this as completed in ea01b9e Jan 15, 2024
@ajcvickers ajcvickers modified the milestones: Backlog, 8.0.0-preview1, 9.0.0-preview1 Mar 26, 2024
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 15, 2024
@roji roji modified the milestones: 9.0.0-preview1, 9.0.0 Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-groupby area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-bug
Projects
None yet
4 participants