From 57a8325fc3177ba0244cf3851f117e954cb44005 Mon Sep 17 00:00:00 2001 From: Maurycy Markowski Date: Tue, 14 Mar 2023 13:39:20 -0700 Subject: [PATCH] Fix to #30358 - Duplicate table alias in generated select query (An item with the same key has already been added) We were not updating usedAliases list as we uniquify tabke aliases after combining two sources because of JOIN. If the resulting query also needs owned type expanded (when owned type is mapped to a separate table - this expansion happens in translation rather than nav expansion), additional table generated could have incorrect alias. Fix is to update the usedAliases list as we make changes to aliases, so that when new tables are added we generate new aliases correctly. Fixes #30358 --- .../SqlExpressions/SelectExpression.Helper.cs | 17 +++- .../OwnedEntityQueryRelationalTestBase.cs | 85 +++++++++++++++++++ .../Query/OwnedEntityQuerySqlServerTest.cs | 17 ++++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs index 93f15cc32be..ec9dd339dd7 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs @@ -396,7 +396,22 @@ public AliasUniquifier(HashSet usedAliases) { for (var i = 0; i < innerSelectExpression._tableReferences.Count; i++) { - var newAlias = GenerateUniqueAlias(_usedAliases, innerSelectExpression._tableReferences[i].Alias); + var currentAlias = innerSelectExpression._tableReferences[i].Alias; + var newAlias = GenerateUniqueAlias(_usedAliases, currentAlias); + + if (newAlias != currentAlias) + { + // we keep the old alias in the list (even though it's not actually being used anymore) + // to disambiguate the APPLY case, e.g. something like this: + // SELECT * FROM EntityOne as e + // OUTER APPLY ( + // SELECT * FROM EntityTwo as e1 + // LEFT JOIN EntityThree as e ON (...) -- reuse alias e, since we use e1 after uniqification + // WHERE e.Foo == e1.Bar -- ambiguity! e could refer to EntityOne or EntityThree + // ) as t + innerSelectExpression._usedAliases.Add(newAlias); + } + innerSelectExpression._tableReferences[i].Alias = newAlias; UnwrapJoinExpression(innerSelectExpression._tables[i]).Alias = newAlias; } diff --git a/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs index 651a9911c21..54e78c7f789 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/OwnedEntityQueryRelationalTestBase.cs @@ -369,6 +369,91 @@ public class Rut public int? Value { get; set; } } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task Join_selects_with_duplicating_aliases_and_owned_expansion_uniquifies_correctly(bool async) + { + var contextFactory = await InitializeAsync(seed: c => c.Seed()); + using var context = contextFactory.CreateContext(); + + var query = from monarch in context.Monarchs + join magus in context.Magi.Where(x => x.Name.Contains("Bayaz")) on monarch.RulerOf equals magus.Affiliation + select new { monarch, magus }; + + var result = async ? await query.ToListAsync() : query.ToList(); + + Assert.Single(result); + Assert.Equal("The Union", result[0].monarch.RulerOf); + Assert.Equal("The Divider", result[0].magus.ToolUsed.Name); + } + + protected class MyContext30358 : DbContext + { + public DbSet Monarchs { get; set; } + public DbSet Magi { get; set; } + + public MyContext30358(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity().OwnsOne(x => x.ToolUsed, x => x.ToTable("MagicTools")); + } + + public void Seed() + { + Add(new Monarch30358 + { + Name = "His August Majesty Guslav the Fifth", + RulerOf = "The Union", + }); + + Add(new Monarch30358 + { + Name = "Emperor Uthman-ul-Dosht", + RulerOf = "The Gurkish Empire", + }); + + Add(new Magus30358 + { + Name = "Bayaz, the First of the Magi", + Affiliation = "The Union", + ToolUsed = new MagicTool30358 { Name = "The Divider" } + }); + + Add(new Magus30358 + { + Name = "The Prophet Khalul", + Affiliation = "The Gurkish Empire", + ToolUsed = new MagicTool30358 { Name = "The Hundred Words" } + }); + + SaveChanges(); + } + } + + public class Monarch30358 + { + public int Id { get; set; } + public string Name { get; set; } + public string RulerOf { get; set; } + } + + public class Magus30358 + { + public int Id { get; set; } + public string Name { get; set; } + public string Affiliation { get; set; } + public MagicTool30358 ToolUsed { get; set; } + } + + public class MagicTool30358 + { + public string Name { get; set; } + } + protected override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder) => base.AddOptions(builder).ConfigureWarnings( c => c diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs index 1d2db57a491..804c9ea3db0 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/OwnedEntityQuerySqlServerTest.cs @@ -178,6 +178,23 @@ public override async Task Owned_entity_with_all_null_properties_property_access """ SELECT [r].[Rot_ApartmentNo] FROM [RotRutCases] AS [r] +"""); + } + + public override async Task Join_selects_with_duplicating_aliases_and_owned_expansion_uniquifies_correctly(bool async) + { + await base.Join_selects_with_duplicating_aliases_and_owned_expansion_uniquifies_correctly(async); + + AssertSql( +""" +SELECT [m].[Id], [m].[Name], [m].[RulerOf], [t].[Id], [t].[Affiliation], [t].[Name], [t].[Magus30358Id], [t].[Name0] +FROM [Monarchs] AS [m] +INNER JOIN ( + SELECT [m0].[Id], [m0].[Affiliation], [m0].[Name], [m1].[Magus30358Id], [m1].[Name] AS [Name0] + FROM [Magi] AS [m0] + LEFT JOIN [MagicTools] AS [m1] ON [m0].[Id] = [m1].[Magus30358Id] + WHERE [m0].[Name] LIKE N'%Bayaz%' +) AS [t] ON [m].[RulerOf] = [t].[Affiliation] """); } }