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

Query: we should propagate optional navigations thru nav rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty #7975

Closed
maumar opened this issue Mar 23, 2017 · 2 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Mar 23, 2017

Repro:

    class Program
    {
        static void Main(string[] args)
        {
            using (var ctx = new MyContext())
            {
                ctx.Database.EnsureDeleted();
                ctx.Database.EnsureCreated();

                var users3 = (from user in ctx.Users
                              join ou in ctx.OrganisationUsers on user.Id equals ou.UserId into link
                              from organisation in link.DefaultIfEmpty()
                              select new
                              {
                                  Id = user.Id,
                                  Organisation = organisation == null ? "N/A" : organisation.Organisation.Name
                              }).ToList();
            }
        }
    }

    public class MyContext : DbContext
    {
        public DbSet<User> Users { get; set; }
        public DbSet<Organisation> Organisations { get; set; }
        public DbSet<OrganisationUser> OrganisationUsers { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<OrganisationUser>().HasKey(ou => new { ou.OrganisationId, ou.UserId });
            modelBuilder.Entity<OrganisationUser>().HasOne(ou => ou.Organisation).WithMany(o => o.OrganisationUsers).HasForeignKey(ou => ou.OrganisationId);
            modelBuilder.Entity<OrganisationUser>().HasOne(ou => ou.User).WithMany(u => u.OrganisationUsers).HasForeignKey(ou => ou.UserId);
        }

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

    public class User
    {
        public int Id { get; set; }
        public List<OrganisationUser> OrganisationUsers { get; set; }
    }

    public class Organisation
    {
        public int Id { get; set; }
        public string Name { get; set; }
        public List<OrganisationUser> OrganisationUsers { get; set; }
    }

    public class OrganisationUser
    {
        public int OrganisationId { get; set; }
        public Organisation Organisation { get; set; }

        public int UserId { get; set; }
        public User User { get; set; }
    }

This produces the following sql:

SELECT [user].[Id], CASE
    WHEN [ou].[OrganisationId] IS NULL
    THEN N'N/A' ELSE [organisation.Organisation].[Name]
END
FROM [Users] AS [user]
LEFT JOIN [OrganisationUsers] AS [ou] ON [user].[Id] = [ou].[UserId]
INNER JOIN [Organisations] AS [organisation.Organisation] ON [ou].[OrganisationId] = [organisation.Organisation].[Id]

Instead we should detect that organization is optional created via SelectMany-GroupJoin-DefaultIfEmpty and create LEFT JOIN instead

@maumar
Copy link
Contributor Author

maumar commented Mar 23, 2017

Found when investigating #7830

@ajcvickers ajcvickers added this to the 2.0.0 milestone Mar 24, 2017
@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Apr 19, 2017
maumar added a commit that referenced this issue May 13, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 13, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 13, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 13, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 14, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 15, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 15, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 16, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).
maumar added a commit that referenced this issue May 16, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty
also fix to #8254 - Null semantics are not carried from subquery

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).

Problem with #8254 was that when eliminating null propagation pattern during SqlTranslation (a != null ? a.Name : null) we were not marking the resulting a.Name as nullable, which then resulted in null semantics not kicking in.
maumar added a commit that referenced this issue May 18, 2017
…av rewrite, if the initial optional navigation was created using SelectMany-GroupJoin-DefaultIfEmpty

also fix to #7761 - Query : Missing null propagation logic for some complex queries with manually created GroupJoin-SelectMany-DefaultIfEmpty
also fix to #8254 - Null semantics are not carried from subquery

Problem for queries with required navigations that were originating from qsre that was a GroupJoin-SelectMany-DefaultIfEmpty. In those cases, we should be producing LEFT JOINs for the navigations, even though they are required (because initial qsre is already "optional"). We would produce INNER JOINs instead, which would result in data corruption - returning fewer results than expected.

Fix is to use more sophisticated logic to determine whether the source qsre is "optional" and therefore we need to expand all its navigations to LOJs.

We also apply this logic to MemberAccess and EF.Property, effectively fully protecting against accidental NREs for InMemory and client eval scenarios (we were doing that before only for results of nav rewrite, now we also do it for manually created LOJs).

Problem with #8254 was that when eliminating null propagation pattern during SqlTranslation (a != null ? a.Name : null) we were not marking the resulting a.Name as nullable, which then resulted in null semantics not kicking in.
@maumar
Copy link
Contributor Author

maumar commented May 18, 2017

fixed in bb7eebd

@maumar maumar closed this as completed May 18, 2017
@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 May 18, 2017
@maumar maumar modified the milestones: 2.0.0-preview2, 2.0.0 May 18, 2017
@ajcvickers ajcvickers modified the milestones: 2.0.0-preview2, 2.0.0 Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

2 participants