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 apply projection for complex properties #32911

Closed
tNRevan opened this issue Jan 24, 2024 · 10 comments · Fixed by #33020
Closed

Incorrect apply projection for complex properties #32911

tNRevan opened this issue Jan 24, 2024 · 10 comments · Fixed by #33020
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 Servicing-approved type-bug
Milestone

Comments

@tNRevan
Copy link

tNRevan commented Jan 24, 2024

When I combine ComplexProperty and AsSplitQuery with properties mapped to column with equal names, I get wrong values when reading the complex properties. Their values are equal (but they are different instances). If I don't use the AsSplitQuery or rename the columns, the results are correct. See the snippet below.

The code is using Microsoft.EntityFrameworkCore.Sqlite 8.0.1 package.

#nullable enable

void Main()
{
	using (var ctx = new MyContext())
	{
		ctx.Database.EnsureDeleted();
		ctx.Database.EnsureCreated();
		ctx.Add(new Offer 
		{
			Variations = [
				new Variation {
					Payment = new Payment(120, 100),
					Nested = new NestedEntity
					{
						Payment = new Payment(12, 10),
					},
					Nested2 = new NestedEntity2
					{
						Payment = new Payment(24, 20),
					},
				}
			]
		});
		ctx.SaveChanges();
		ctx.ChangeTracker.Clear(); // If I don't clear the change tracker, it also works.
		
		var entity = ctx.Offers
			.Include(e => e.Variations!)
				.ThenInclude(v => v.Nested)
			.Include(e => e.Variations!)
				.ThenInclude(v => v.Nested2)
			.AsSplitQuery() // If I comment this out, it works
			.Single(e => e.Id == 1);
		
		//  The values are equal even when they shouldn't be.
		Console.WriteLine($"Entity {entity.Variations?.Single().Payment}");
		Console.WriteLine($"Nested Entity {entity.Variations?.Single().Nested?.Payment}");
		Console.WriteLine($"Nested Entity2 {entity.Variations?.Single().Nested2?.Payment}");
	}
}

public class MyContext : DbContext
{
	public DbSet<Offer> Offers => Set<Offer>();
	
	protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
	{
	    optionsBuilder.UseSqlite("DataSource=Test.db");
	}
	
	protected override void OnModelCreating(ModelBuilder builder)
	{
		builder.Entity<Variation>().ComplexProperty(p => p.Payment, price =>
		{
		    price.Property(p => p.Netto).HasColumnName("payment_netto"); // If I change the column name to a different than the others, it also works
		    price.Property(p => p.Brutto).HasColumnName("payment_brutto");
		});
		builder.Entity<NestedEntity>().ComplexProperty(p => p.Payment, price =>
		{
		    price.Property(p => p.Netto).HasColumnName("payment_netto");
		    price.Property(p => p.Brutto).HasColumnName("payment_brutto");
		});
		builder.Entity<NestedEntity2>().ComplexProperty(p => p.Payment, price =>
		{
		    price.Property(p => p.Netto).HasColumnName("payment_netto");
		    price.Property(p => p.Brutto).HasColumnName("payment_brutto");
		});
	}
}

public abstract class EntityBase 
{
	[System.ComponentModel.DataAnnotations.Key]
	public int Id {get; set; }
}

public class Offer : EntityBase
{	
	public ICollection<Variation>? Variations { get; set; }
}

public class Variation : EntityBase
{
	public Payment Payment {get; set; } = new Payment(0, 0);

	public NestedEntity? Nested {get; set; }
	public NestedEntity2? Nested2 {get; set; }
}

public class NestedEntity : EntityBase
{
	public Payment Payment {get; set; } = new Payment(0, 0);
}

public class NestedEntity2 : EntityBase
{
	public Payment Payment {get; set; } = new Payment(0, 0);
}

public record Payment(decimal Netto, decimal Brutto);

Include provider and version information

EF Core version: 8.0.1
Database provider: Microsoft.EntityFrameworkCore.Sqlite 8.0.1
Target framework: .NET 8.0
Operating system: Windows 10
IDE: Visual Studio 2022 17.8.5

@ajcvickers
Copy link
Member

Still repros on latest daily. /cc @roji @maumar

@maumar
Copy link
Contributor

maumar commented Feb 1, 2024

Simplified:

#nullable enable

    [ConditionalFact]
    public void Repro32911()
    {
        using var ctx = new MyContext();
        ctx.Database.EnsureDeleted();
        ctx.Database.EnsureCreated();
        ctx.Add(new Offer
        {
            Variations = [
                new Variation
                {
                    Payment = new Payment(120, 100),
                    Nested = new NestedEntity
                    {
                        Payment = new Payment(12, 10),
                    },
                }
            ]
        });

        ctx.SaveChanges();
        ctx.ChangeTracker.Clear();

        var entity = ctx.Offers
            .Include(e => e.Variations!)
                .ThenInclude(v => v.Nested)
            .AsSplitQuery()
            .Single(e => e.Id == 1);

        var one = entity.Variations?.Single().Payment;
        var two = entity.Variations?.Single().Nested?.Payment;
        if (one == two) throw new InvalidOperationException("wrong");
    }

    public class MyContext : DbContext
    {
        public DbSet<Offer> Offers => Set<Offer>();

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Repro32911;Trusted_Connection=True;MultipleActiveResultSets=true");
        }

        protected override void OnModelCreating(ModelBuilder builder)
        {
            builder.Entity<Variation>().ComplexProperty(p => p.Payment, price =>
            {
                price.Property(p => p.Netto).HasColumnName("payment_netto");
                price.Property(p => p.Brutto).HasColumnName("payment_brutto");
            });
            builder.Entity<NestedEntity>().ComplexProperty(p => p.Payment, price =>
            {
                price.Property(p => p.Netto).HasColumnName("payment_netto");
                price.Property(p => p.Brutto).HasColumnName("payment_brutto");
            });
        }
    }

    public abstract class EntityBase
    {
        [System.ComponentModel.DataAnnotations.Key]
        public int Id { get; set; }
    }

    public class Offer : EntityBase
    {
        public ICollection<Variation>? Variations { get; set; }
    }

    public class Variation : EntityBase
    {
        public Payment Payment { get; set; } = new Payment(0, 0);

        public NestedEntity? Nested { get; set; }
    }

    public class NestedEntity : EntityBase
    {
        public Payment Payment { get; set; } = new Payment(0, 0);
    }

    public record Payment(decimal Netto, decimal Brutto);

#nullable disable

@maumar
Copy link
Contributor

maumar commented Feb 1, 2024

query we generate is wrong:

SELECT TOP(2) [o].[Id]
FROM [Offers] AS [o]
WHERE [o].[Id] = 1
ORDER BY [o].[Id]

SELECT [s].[Id], [s].[NestedId], [s].[OfferId], [s].[payment_brutto], [s].[payment_netto], [s].[Id0], [o0].[Id]
FROM (
    SELECT TOP(1) [o].[Id]
    FROM [Offers] AS [o]
    WHERE [o].[Id] = 1
) AS [o0]
INNER JOIN (
    SELECT [v].[Id], [v].[NestedId], [v].[OfferId], [v].[payment_brutto], [v].[payment_netto], [n].[Id] AS [Id0]
    FROM [Variation] AS [v]
    LEFT JOIN [NestedEntity] AS [n] ON [v].[NestedId] = [n].[Id]
) AS [s] ON [o0].[Id] = [s].[OfferId]
ORDER BY [o0].[Id]

we only project brutto and netto values from Variation entity and don't project those from NestedEntity.

As comparison, single query generates this:

SELECT [o0].[Id], [s].[Id], [s].[NestedId], [s].[OfferId], [s].[payment_brutto], [s].[payment_netto], [s].[Id0], [s].[payment_brutto0], [s].[payment_netto0]
FROM (
    SELECT TOP(2) [o].[Id]
    FROM [Offers] AS [o]
    WHERE [o].[Id] = 1
) AS [o0]
LEFT JOIN (
    SELECT [v].[Id], [v].[NestedId], [v].[OfferId], [v].[payment_brutto], [v].[payment_netto], [n].[Id] AS [Id0], [n].[payment_brutto] AS [payment_brutto0], [n].[payment_netto] AS [payment_netto0]
    FROM [Variation] AS [v]
    LEFT JOIN [NestedEntity] AS [n] ON [v].[NestedId] = [n].[Id]
) AS [s] ON [o0].[Id] = [s].[OfferId]
ORDER BY [o0].[Id], [s].[Id]

@maumar
Copy link
Contributor

maumar commented Feb 1, 2024

problem is during apply projection. In GenerateComplexPropertyShaperExpression we loop through properties of the complex type to generate corresponding column, but we don't have enough information to create the correct column. We just take table alias and name from IColumn. However in our case the same column is projected multiple times (and aliased accoringly):

SELECT v.Id, v.NestedId, v.OfferId, v.payment_brutto, v.payment_netto, n.Id AS Id0, n.payment_brutto AS payment_brutto0, n.payment_netto AS payment_netto0
    FROM Variation AS v
    LEFT JOIN NestedEntity AS n ON v.NestedId == n.Id

so when we generate projection for the second complex type NestedEntity.Payment we associate it with payment_brutto and payment_netto columns rather than payment_brutto0 and payment_netto0. If the columns for both types have different names we generate distinct ones and everything works.

@ajcvickers ajcvickers added this to the 8.0.x milestone Feb 2, 2024
maumar added a commit that referenced this issue Feb 6, 2024
maumar added a commit that referenced this issue Feb 7, 2024
Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations.

Fixes #32911
@maumar maumar added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels Feb 7, 2024
maumar added a commit that referenced this issue Feb 8, 2024
Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes #32911
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 8, 2024
maumar added a commit that referenced this issue Feb 8, 2024
Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes #32911
@maumar maumar removed this from the 8.0.x milestone Feb 8, 2024
@ajcvickers ajcvickers added this to the 9.0.0 milestone Feb 9, 2024
maumar added a commit that referenced this issue Feb 13, 2024
Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes #32911
@maumar maumar removed this from the 9.0.0 milestone Feb 13, 2024
@maumar
Copy link
Contributor

maumar commented Feb 13, 2024

reopening for possible patch

@maumar maumar reopened this Feb 13, 2024
@solistice
Copy link

solistice commented Feb 14, 2024

Does this fix include occurrences where AsSplitQuery is not used? We encountered this bug as well without using AsSplitQuery. I looked at the commit but couldn't see if this was specific to the AsSplitQuery case, but all the comments and pointers state only that case. Happy to create and share some runnable code to show what I mean

@roji
Copy link
Member

roji commented Feb 14, 2024

@solistice yeah, it should. Can you please try the latest daily build and confirm that it fixes your case? If it doesn't, can you please open a new issue with a minimal, runnable repo?

@solistice
Copy link

I can confirm that the latest daily build fixes our issue as well! Thanks!

@roji
Copy link
Member

roji commented Feb 14, 2024

Thanks @solistice!

@roji roji changed the title ComplexProperty with AsSplitQuery Incorrect apply projection for complex properties Feb 28, 2024
@ajcvickers ajcvickers added this to the 8.0.x milestone Feb 28, 2024
maumar added a commit that referenced this issue Feb 29, 2024
Additional test for #32911 plus a small code correction
maumar added a commit that referenced this issue Mar 1, 2024
Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes #32911
maumar added a commit that referenced this issue Mar 1, 2024
Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes #32911
maumar added a commit that referenced this issue Mar 1, 2024
maumar added a commit that referenced this issue Mar 1, 2024
maumar added a commit that referenced this issue Mar 3, 2024
Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes #32911
maumar added a commit that referenced this issue Mar 7, 2024
…3212)

Problem was that when we lift structural type projection during pushdown, if that projection contains complex types with the same column names (e.g. cross join of same entities - it's perfectly fine to do in a vacuum, we just alias the columns whose names are repeated) we would lift the projection incorrectly.
What we do is go through all the properties, apply the corresponding columns to the selectExpression if needed and generate StructuralTypeProjection object if the projection needs to be applied one level up. For complex types we would generate a shaper expression and then run it through the same process, BUT the nested complex properties would be added to a flat structure along with the primitive properties, rather than in separate cache dedicated for complex property shapers.
This was wrong and not what we expected to see, when processing this structure one level up (i.e. when applying projection to the outer select)

SELECT [applying_this_projection_was_wrong]
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

i.e. applying projection once worked fine, but doing it second time did not. The reason why is that we expected to see information about the complex type shape in the complex property shaper cache, rather than flat structure for primitives, but it wasn't there. So we assumed this is the first time we the projection is being applied, so we conjure up the complex type shaper based on table alias and IColumn metadata. This results in a situation, where complex property that was aliased is never picked. So we end up with:

SELECT s.Id, s.Name, s.ComplexProp -- we would also try to add s.ComplexProp again, instead of s.ComplexProp0 but of course we don't add same thing twice
FROM
(
    SELECT c.Id, c.Name, c.ComplexProp, o.ComplexProp as ComplexProp0
    FROM Customers as c
    JOIN Orders as o ON ...
) as s

This leads to bad data - two different objects with distinct data in them are mapped to the same column in the database.

Fix is to property build a complex type shaper structure when applying projection instead, so the structure we generate matches expectations. Also modified VisitChildren and MakeNullable methods on StructuralTypeProjectionExpression to process/preserve complex type cache information, which was previously gobbled up/ignored.

Fixes #32911
@maumar
Copy link
Contributor

maumar commented Mar 7, 2024

This issue has been approved for patch and will be available in 8.0.4, closing

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 Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants