Skip to content

Filtered Foreign Key Attribute #21991

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
tanekim88 opened this issue Aug 8, 2020 · 14 comments
Closed

Filtered Foreign Key Attribute #21991

tanekim88 opened this issue Aug 8, 2020 · 14 comments

Comments

@tanekim88
Copy link

tanekim88 commented Aug 8, 2020

In Entity Framework Core 5, "filtered include" feature is finally scheduled to be released.
Now, we need some love for ForeignKey attribute as well. It would be great if I can do something like the following with the ForeignKey attribute. I believe implementing this feature is easily doable using the "filtered include" feature.

    class ChildGroup
    {
        [Key]
        public int ChildGroupId { get; set; }
        public ICollection<User> ChildUsers { get; set; }
    }

    class User
    {
        [Key]
        public int UserId { get; set; }

        public int Age { get; set; }

        public int ChildGroupId { get; set; }

        [ForeignKey("ChildGroupId", user => user.Age < 14)]
        public ChildGroup ChildGroup { get; set; }
    }

This will give more flexibility in mapping relationships through ForeignKey attributes.

@ajcvickers
Copy link
Contributor

@tanekim77 Leaving syntax aside for the moment, it's not clear exactly what you expect to happen when user => user.Age < 14 is applied to the FK. Is the intention to create a filtered index associated with the FK columns? Or is intended to be sugar for always performing a filtered Include when using the associated navigation in an Include? If so, is this only applied when using eager loading, or when the relationship is loaded in any way?

@tanekim88
Copy link
Author

tanekim88 commented Aug 10, 2020

@ajcvickers
I was thinking it would be for both eager and lazy loading and when the relationship is loaded in any way.
I.e., _context.ChildGroup.Include(cg=>cg.ChildUsers).ToList() would actually perform filtered include behind the scene =>_context.ChildGroup.Include(cg=>cg.ChildUsers.Where(user => user.Age < 14)).ToList().

@smitpatel
Copy link
Contributor

@tanekim77 - If it is for include and specified via ForeignKeyAttribute then would you expect it to be applied as filter on navigation

  • Principal to dependent navigation only
  • Dependent to principal navigation only
  • both navigations
  • Only collection navigation if any

Would it make more sense for the configuration of filter to be tied with navigation in a different way rather than ForeignKeyAttribute?

@tanekim88
Copy link
Author

tanekim88 commented Aug 10, 2020

@smitpatel
I am thinking that the 2nd argument for ForeignKey Attribute should only be allowed for one to many relationships, otherwise it should show syntax error, and filtered include would only be applied to "Principal to dependent navigation only".
For many to many relationship, it should be configured through Fluent Api. For example, if OldStudentDifficultyCourse is the joining table for tables Student and Course for many to many relationship,


modelBuilder.Entity<OldStudentDifficultCourse>().HasKey(sc => new { sc.SId, sc.CId });

modelBuilder.Entity<OldStudentDifficultCourse>()
    .HasOne<Student>(sc => sc.Student)
    .WithMany(s => s.OldStudentDifficultCourses.Where(sc=> sc.student.Age > 30))
    .HasForeignKey(sc => sc.SId);


modelBuilder.Entity<OldStudentDifficultCourse>()
    .HasOne<Course>(sc => sc.Course)
    .WithMany(s => s.OldStudentDifficultCourses.Where(sc=> sc.course.Difficulty > 10))
    .HasForeignKey(sc => sc.CId);

In this case, if a student who has age < 31 is trying to include OldStudentDifficultCourses navigation property, then it would return null.

@smitpatel
Copy link
Contributor

Given we have NavigationBuilder, it would be better to put them on NavigationBuilder.

modelBuilder.Entity<Student>().Navigation(e => e.Courses).HasIncludeFilter(e => e.Where(course => course.Difficulty >10));

@AndriySvyryd - Do we need a generic Navigation Builder now?

@AndriySvyryd
Copy link
Member

@smitpatel Sure - #22012

@tanekim88
Copy link
Author

tanekim88 commented Aug 11, 2020

As for the attribute notations, for either many-to-many or one-to-many, what do you think about the following notation?

For one-to-many:

public class ChildGroup
  {
      [Key]
      public int ChildGroupId { get; set; }

      [Where(user => user.Age < 14)]
      public virtual ICollection<User> ChildUsers { get; set; }
  }

public class User
  {
      [Key]
      public int UserId { get; set; }

      public int Age { get; set; }

      public int ChildGroupId { get; set; }

      [ForeignKey("ChildGroupId")]
      public virtual ChildGroup ChildGroup { get; set; }
  }

For many-to-many:

    public class Student {
        [Where(od => od.Course.Difficulty > 10)]
        public virtual ICollection<OldStudentDifficultCourse> OldStudentDifficultCourses { get; set; }
    }

    public class Course {
        [Where(od => od.Student.Age > 30)]
        public virtual ICollection<OldStudentDifficultCourse> OldStudentDifficultCourses { get; set; }
    }

followed by the standard many-to-many Fluent api configuration.

Or alternatively and optionally, one can choose to define both criteria on both sides just to be more clear,

    public class Student {
        [Where(od => od.Student.Age > 30 && od.Course.Difficulty > 10)]
        public virtual ICollection<OldStudentDifficultCourse> OldStudentDifficultCourses { get; set; }
    }

    public class Course {
        [Where(od => od.Student.Age > 30 && od.Course.Difficulty > 10)]
        public virtual ICollection<OldStudentDifficultCourse> OldStudentDifficultCourses { get; set; }
    }

@smitpatel
Copy link
Contributor

It would be more useful to call it IncludeFilterAttribute may be rather than Where.
Where is ambiguous in many ways. And EF Core include filters support Where/OrderBy/Skip/Take.

Regardless, before we add any attribute to configure it, we will first need to add a fluent API way for it and design whole feature. Attributes can be tricky depending on what autocompletion it provides. We are talking about writing a LINQ in an attributes ctor here. Without proper intellisense support attribute would not provide much value.

@roji
Copy link
Member

roji commented Aug 11, 2020

Just a reminder that attributes are extremely limited in terms of parameter types. AFAIK C# simply does not support lambdas in attributes.

However as @smitpatel suggested this could be a fluent API, which would be somewhat similar to AutoInclude API, specifying that when a navigation is included, it should be implicitly filtered.

@tanekim88
Copy link
Author

@roji

I am seeing the silver lining in the cloud at least in the distant future in the following link:
https://github.com/dotnet/csharplang/blob/master/proposals/lambda-attributes.md

Hopefully, if this goes through approval, then the filtered attribute feature may one day be a dream come true.

@roji
Copy link
Member

roji commented Aug 11, 2020

@tanekim77 I think that issue is to allow applying attributes to lambda, rather than to allow lambdas to be passed as arguments to attributes

@tanekim88
Copy link
Author

@roji

Oops, facepalm. I think it's a right time to say goodbye to my pipe dream.

@ajcvickers
Copy link
Contributor

Discussed in triage and decided that this is really about being able to map a navigation to some different backing query (which may do filtering.) This is tracked by #20339

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

6 participants
@ajcvickers @smitpatel @roji @AndriySvyryd @tanekim88 and others