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

Materialization condition for optional dependents with all null properties but having own optional dependent #21488

Closed
smitpatel opened this issue Jul 2, 2020 · 2 comments · Fixed by #24573
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@smitpatel
Copy link
Contributor

Using 5.0 preview6
Repro code:

using System;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace EFSampleApp
{
    public class Program
    {
        public static async Task Main(string[] args)
        {
            using (var db = new MyContext())
            {
                // Recreate database
                db.Database.EnsureDeleted();
                db.Database.EnsureCreated();

                // Seed database
                db.Add(new Vehicle
                {
                    VehicleName = "Car",
                    Operator = new Operator
                    {
                        OperatorDetails = new OperatorDetails
                        {
                            Details = "Dull"
                        }
                    }
                });

                db.SaveChanges();
            }

            using (var db = new MyContext())
            {
                // Run queries
                var query = db.Set<Vehicle>().Include(e => e.Operator.OperatorDetails).First();

                Console.WriteLine(query.VehicleName);
                Console.WriteLine(query.Operator);
                Console.WriteLine(query.Operator.OperatorName);
                Console.WriteLine(query.Operator.OperatorDetails);
                Console.WriteLine(query.Operator.OperatorDetails.Details);
            }
            Console.WriteLine("Program finished.");
        }
    }


    public class MyContext : DbContext
    {
        private static ILoggerFactory ContextLoggerFactory
            => LoggerFactory.Create(b =>
            {
                b
                .AddConsole()
                .AddFilter("", LogLevel.Debug);
            });

        // Declare DBSets
        public DbSet<Vehicle> Vehicles { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            // Select 1 provider
            optionsBuilder
                .UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=_ModelApp;Trusted_Connection=True;Connect Timeout=5;ConnectRetryCount=0")
                //.UseSqlite("filename=_modelApp.db")
                //.UseInMemoryDatabase(databaseName: "_modelApp")
                //.UseCosmos("https://localhost:8081", @"C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==", "_ModelApp")
                .EnableSensitiveDataLogging()
                .UseLoggerFactory(ContextLoggerFactory);
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            // Configure model
        }
    }

    public class Vehicle
    {
        public int Id { get; set; }
        public string VehicleName { get; set; }
        public Operator Operator { get; set; }
    }

    [Table("Vehicles")]
    public class Operator
    {
        public int Id { get; set; }
        public string OperatorName { get; set; }
        [ForeignKey("Id")]
        public Vehicle Vehicle { get; set; }
        public OperatorDetails OperatorDetails { get; set; }
    }

    [Table("Vehicles")]
    public class OperatorDetails
    {
        public int Id { get; set; }
        public string Details { get; set; }
        [ForeignKey("Id")]
        public Operator Operator { get; set; }
    }
}

We generate following SQL,

      SELECT TOP(1) [v].[Id], [v].[VehicleName], [t1].[Id], [t1].[OperatorName], [t5].[Id], [t5].[Details]
      FROM [Vehicles] AS [v]
      LEFT JOIN (
          SELECT [t0].[Id], [t0].[OperatorName], [v3].[Id] AS [Id0]
          FROM (
              SELECT [v0].[Id], [v0].[OperatorName]
              FROM [Vehicles] AS [v0]
              WHERE [v0].[OperatorName] IS NOT NULL
              UNION
              SELECT [v1].[Id], [v1].[OperatorName]
              FROM [Vehicles] AS [v1]
              INNER JOIN (
                  SELECT [v2].[Id], [v2].[Details]
                  FROM [Vehicles] AS [v2]
                  WHERE [v2].[Details] IS NOT NULL
              ) AS [t] ON [v1].[Id] = [t].[Id]
          ) AS [t0]
          INNER JOIN [Vehicles] AS [v3] ON [t0].[Id] = [v3].[Id]
      ) AS [t1] ON [v].[Id] = [t1].[Id]
      LEFT JOIN (
          SELECT [v4].[Id], [v4].[Details], [t4].[Id] AS [Id0], [t4].[Id0] AS [Id00]
          FROM [Vehicles] AS [v4]
          INNER JOIN (
              SELECT [t3].[Id], [t3].[OperatorName], [v8].[Id] AS [Id0]
              FROM (
                  SELECT [v5].[Id], [v5].[OperatorName]
                  FROM [Vehicles] AS [v5]
                  WHERE [v5].[OperatorName] IS NOT NULL
                  UNION
                  SELECT [v6].[Id], [v6].[OperatorName]
                  FROM [Vehicles] AS [v6]
                  INNER JOIN (
                      SELECT [v7].[Id], [v7].[Details]
                      FROM [Vehicles] AS [v7]
                      WHERE [v7].[Details] IS NOT NULL
                  ) AS [t2] ON [v6].[Id] = [t2].[Id]
              ) AS [t3]
              INNER JOIN [Vehicles] AS [v8] ON [t3].[Id] = [v8].[Id]
          ) AS [t4] ON [v4].[Id] = [t4].[Id]
          WHERE [v4].[Details] IS NOT NULL
      ) AS [t5] ON [t1].[Id] = [t5].[Id]

Issue: Vehicle -> Operator -> OperatorDetails are sharing table here. All the non-pk properties of Operator are nullable.
While generating SQL we generate Union with the dependents (to include operators which have associated OperatorDetails) regardless of values in Operator columns. But when we are materializing we don't have that information. We only have ValueBuffer associated with Operator and which has PK with non-null value and rest is null values. PK is expected to non-null due to table splitting. Hence we don't materialize the instance and above throws null ref.

@ajcvickers ajcvickers added this to the Backlog milestone Jul 6, 2020
@ajcvickers ajcvickers modified the milestones: Backlog, 6.0.0 Nov 5, 2020
@smitpatel smitpatel added blocked closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed needs-design blocked labels Dec 3, 2020
smitpatel added a commit that referenced this issue Dec 9, 2020
An optional dependent considered present
If all non-nullable non-PK properties have a non-null value, or
If all non-nullable non-PK properties are shared with principal then at least one nullable non shared property has a non-null value
If there are no non-shared properties then it is always present and act like a required dependent

Resolves #23564
Resolves #21488
Resolves #23198
smitpatel added a commit that referenced this issue Dec 9, 2020
An optional dependent considered present
If all non-nullable non-PK properties have a non-null value, or
If all non-nullable non-PK properties are shared with principal then at least one nullable non shared property has a non-null value
If there are no non-shared properties then it is always present and act like a required dependent

Resolves #23564
Resolves #21488
Resolves #23198
@smitpatel
Copy link
Contributor Author

We decided that we are going to throw for this scenario and user must add a required property in the middle dependent which can serve as identifying property to determine materialization.

@scottk4106
Copy link

We decided that we are going to throw for this scenario and user must add a required property in the middle dependent which can serve as identifying property to determine materialization.

Just looking for a little more detail. Does this mean that if this situation occurs, an exception will be thrown rather than not returning the data expected?

To add a required property as suggested, can you change the Operator class and add the property Ignore, like this?

    [Table("Vehicles")]
    public class Operator
    {
        public int Id { get; set; }
        public string OperatorName { get; set; }
        [ForeignKey("Id")]
        public Vehicle Vehicle { get; set; }
        public OperatorDetails OperatorDetails { get; set; }

        public int Ignore { get; private set; } = 0;
    }

@ajcvickers ajcvickers modified the milestones: 6.0.0-preview4, 6.0.0 Nov 8, 2021
This issue was closed.
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. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants