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

Support Polymorphic Relationships #7623

Open
Tracked by #240 ...
bjorn-ali-goransson opened this issue Feb 15, 2017 · 20 comments
Open
Tracked by #240 ...

Support Polymorphic Relationships #7623

bjorn-ali-goransson opened this issue Feb 15, 2017 · 20 comments

Comments

@bjorn-ali-goransson
Copy link

bjorn-ali-goransson commented Feb 15, 2017

For example: I have two Entity Types Page and Post, and both implement the interface ITeaser. Let's say that ITeaser has a Title.

Now, I want my Post to have a new property RecommendedNextRead that is of type ITeaser, and can thus be of both type Post and Page.

What would be the best way to implement this? Is it even feasible supported?

@rowanmiller rowanmiller changed the title Reference to Type spanning across several entity types (and therefore tables) Support Polymorphic Relationships Feb 22, 2017
@rowanmiller rowanmiller added this to the Backlog milestone Feb 22, 2017
@rowanmiller
Copy link
Contributor

It's not currently supported, unless Post/Tag have a common base type (but that means they would need to be stored in the same table with TPH... presumably not what you want 😄). We have discussed supporting this, but it's not something we're working on at the moment or in the immediate future. Moving to the backlog.

@bdebaere
Copy link

Copied from my StackOverflow post.

Consider the following table design...

    Order
      Id UNIQUEIDENTIFIER
    
    OrderLine
      Id UNIQUEIDENTIFIER
      OrderId UNIQUEIDENTIFIER
    
    Connection
      Id UNIQUEIDENTIFIER
      FromEntityId UNIQUEIDENTIFIER
      ToEntityId UNIQUEIDENTIFIER

It allows linking any table to any other table via the Connection table. The alternative being to create a ConnectionFromTxToTy table for every single combination. Which in our schema means creating 70! (! for factorial) tables just for this functionality. On top of this that would mean adding 70 navigation properties to every class. This is far from easy to implement or consume.

It is possible to add a navigation property Connections to my models. Which does allow me to view all Connections linked to a single entity (provided I double them up).

    order
        .HasMany(entity => entity.Connections)
        .WithOne()
        .HasForeignKey(entity => entity.FromEntityId)

However, I'd like to go one step further and actually be able to include the linked entity, like any other entity. The downside of course is that this is not supported by EntityFrameworkCore. I cannot create a class like below and expect it to work out of the box.

    public class Connection
    {
        public Guid Id { get; set; }
    
        public Guid FromEntityId { get; set; }
    
        public Guid ToEntityId { get; set; }
    
        public virtual object FromEntity { get; set; }
    
        public virtual object ToEntity { get; set; }
    }

And as soon I would try to include on Orders.Include(order => order.Connections).ThenInclude(connection => connection.ToEntity) I would lose all typed functionality. But this I could live with if something like the following would function: Orders.Include(order => order.Connections).ThenInclude(connection => connection.ToEntity).ThenInclude(orderLine => ((OrderLine)orderLine).Order).

Currently I've included the linked entity as JSON, but that obviously means I lose all include functionality after that. But worse than that, any ValueConverters I assigned to that type aren't being executed.

How can I tackle this challenge? I'm not sure what's the best path: the virtual IDictionary<string, object> route or the virtual object route or something else. I feel like I might potentially get there with a custom QueryModelGenerator and a custom SqlServerQuerySqlGeneratorFactory, although I don't know if that alone would be enough. But EntityFrameworkCore stops me already at the navigation property with type object.

@bjorn-ali-goransson
Copy link
Author

Hey,

Sounds like a relational database isn't the right tool for the job, honestly ...

@bricelam
Copy link
Contributor

See also #757

@atrauzzi
Copy link

I'd definitely like to see polymorphic relations supported and if the number of times it's asked for even in this project's bug tracker is any indicator... It's quite high priority.

@AndriySvyryd
Copy link
Member

@atrauzzi This issue currently only has 9 👍 votes, so it's actually rather low on the feature request list

@atrauzzi
Copy link

@AndriySvyryd - I've been scanning this repository as well as stack overflow (as well as others) and the combined organic demand is a much different story.

I get you're trying to be objective, but that methodology leans into a disingenuous extreme. It takes empathy to truly understand what users want, not the expectation that people find the right place to put a "thumbs up" as well as to self-select in such a specific way.

Hopefully feature priority for this project can be more visionary, not reactionary...

@AndriySvyryd
Copy link
Member

Yes, we take more thing into account for the release planning process, this is just the most accessible metric.

@atrauzzi
Copy link

atrauzzi commented Nov 1, 2020

Right, but I don't want it to become a bad-faith distraction to my point which is that I think demand for this in particular is more than the most accessible metric.

My apologies, but also in my defence, I do have a knack for these things.

@AndriySvyryd
Copy link
Member

We are gathering feedback to make sure that we don't miss any gaps not currently reflected in the issues. We'll adjust our plans if necessary according to the data.

@atrauzzi
Copy link

atrauzzi commented Nov 1, 2020

Hmm. That's really no different to waiting for +1s. It substitutes analysis & intuition with statistics and intellect. This goes back to my previous point about being visionary and not reactionary.

I get it, the next thing to say is that if people don't speak up, they can't be helped... But then what is the long term impact of that and what blind spots does it create? Current sentiment towards EF core is hurting .net in general and quite a few long-term asks have been sitting in the freezer since 2014.

The impression given here is that the data is bad due to a bad methodology and it's pulling focus away from the things people actually want.

@roji
Copy link
Member

roji commented Nov 1, 2020

@atrauzzi we aren't just looking at data - analysis and intuition are also telling us that certain other issues are probably more important to our users (the thing about intuition is that it's a bit hard to substantiate 🤣). I also don't believe in purely data- and metric-driven decisions, but I don't think this project does that in general. It's easy to say that a certain feature is important or has been in the backlog for a long time. What's hard is to gauge its importance compared to everything else that users want.

@ajcvickers
Copy link
Member

Note: see the pattern in #25472.

@atrauzzi
Copy link

Thanks @ajcvickers!

Maybe worth mentioning here that this is similar to how Laravel does its polymorphic relations. And the discriminator that the ORM uses is easily customized.

@tyler-boyd
Copy link

For people that may be blocked on this, there is a possible workaround that might work with your data model:

Have a TeaserContainer model which has optional belongs to relationships with both Post and Page.

Now, Post.RecommendedNextRead can be a belongs to association of type TeaserContainer, and Post can implement a function like ITeaser GetRecommendedNextRead => RecommendedNextRead.Post ?? RecommendedNextRead.Page;

So the database would look like:

TeaserContainer
post_id: int?
page_id: int?

Page
recommended_next_read_id: int?

And in the Page class:

class Page
{
  [ForeignKey("recommended_next_read_id")]
  public TeaserContainer? TeaserContainer { get; set; }
  public ITeaser? RecommendedNextRead => TeaserContainer?.Page ?? TeaserContainer?.Post;
}

@abdumostafa
Copy link

abdumostafa commented Apr 3, 2022

Until fully supported, what is the work around to implement polymorphic relations in EF core like

public class Employee{
public ICollection Documents{get;set;}
}
public abstract class Document{
public int Id{get;set;}
public string Name{get;set;}
}
public class OfficalDocument : Document{
public Guid CitizenId{get;set;}
}
public class RegularDocument : Document{
public string ReferenceNumber{get;set;}
}

@ajcvickers
Copy link
Member

@abdumostafa There shouldn't be any issue mapping those entities now. See my sample code below:

public class Employee
{
    public int Id { get; set; }
    public ICollection<Document> Documents { get; set; }
}

public abstract class Document
{
    public int Id { get; set; }
    public string Name { get; set; }
}

public class OfficialDocument : Document
{
    public Guid CitizenId { get; set; }
}

public class RegularDocument : Document
{
    public string ReferenceNumber { get; set; }
}

public class SomeDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(Your.ConnectionString)
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Employee>();
        modelBuilder.Entity<Document>();
        modelBuilder.Entity<OfficialDocument>();
        modelBuilder.Entity<RegularDocument>();
    }
}

public class Program
{
    public static void Main()
    {
        using (var context = new SomeDbContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.Add(new Employee
            {
                Documents = new List<Document>
                {
                    new RegularDocument {Name = "Reg", ReferenceNumber = "ref1"},
                    new OfficialDocument {Name = "Off", CitizenId = Guid.NewGuid()},
                }
            });
            context.SaveChanges();
        }

        using (var context = new SomeDbContext())
        {
            var employee = context.Set<Employee>().Include(e => e.Documents).Single();
            foreach (var document in employee.Documents.ToList())
            {
                Console.WriteLine(document is OfficialDocument officialDocument
                    ? $"Official: {document.Name} {officialDocument.CitizenId}"
                    : $"Regular: {document.Name} {((RegularDocument)document).ReferenceNumber}");
            }
        }
    }
}

@SoftCircuits
Copy link

SoftCircuits commented Oct 2, 2023

@ajcvickers

@abdumostafa There shouldn't be any issue mapping those entities now. See my sample code below:

I didn't understand your sample code. How is the relationship between Employees and Documents mapped in the database? I don't see any foreign keys.

Also, was anything new added to .NET to better support this since you posted that?

Thanks.

@MoishyS
Copy link

MoishyS commented Nov 9, 2023

I am surprised that it's still not in plan, Laravel has had it for like 10 years already.
https://laravel.com/docs/master/eloquent-relationships#one-to-many-polymorphic-relations

posts
    id - integer
    title - string
    body - text
 
videos
    id - integer
    title - string
    url - string
 
comments
    id - integer
    body - text
    commentable_id - integer
    commentable_type - string

#15854 (comment) would be a workaround but that's also not planned, any workaround for now?

@EmperorArthur
Copy link

I'm going to also request this. Especially since #25141 was rolled into it.

EF Core is supposed to be something usable by large organizations. Yet a consequence of that environment is we often don't get to adjust the DB to what a library expects. DBAs don't like the performance impact of adding a column where every value is the same, and probably see forcing the use of raw SQL and stored procedures as a plus.

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

No branches or pull requests