-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Non-string discriminator properties throw on multiple configuration calls #15553
Comments
Notes for triage: more minimal repro below. Model builds correctly if there are no relationships between the two hierarchies, but throws exception reported when a relationship is introduced. using Microsoft.EntityFrameworkCore;
public class ProgramOneTphToOneTph
{
public static void Main()
{
var options = new DbContextOptionsBuilder()
.UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=Test;Connect Timeout=5")
.Options;
using (var db = new TestContext(options))
{
var model = db.Model;
db.Database.EnsureDeleted();
db.Database.EnsureCreated();
}
}
private enum ParentDiscriminator
{
Good = 1,
Bad = 2
}
private enum ParentChildDiscriminator
{
Good = 1,
Bad = 2
}
private abstract class ParentBase
{
public int Id { get; set; }
public ParentDiscriminator Discriminator { get; protected set; }
}
private class GoodParent : ParentBase
{
}
private class BadParent : ParentBase
{
public BadChild Child { get; set; }
}
private abstract class ChildBase
{
public int Id { get; set; }
public ParentChildDiscriminator Discriminator { get; protected set; }
}
private class GoodChild : ChildBase
{
}
private class BadChild : ChildBase
{
}
public class TestContext : DbContext
{
public TestContext(DbContextOptions options) :
base(options)
{
}
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);
modelBuilder.Entity<ParentBase>()
.HasDiscriminator(x => x.Discriminator)
.HasValue<GoodParent>(ParentDiscriminator.Good)
.HasValue<BadParent>(ParentDiscriminator.Bad);
modelBuilder.Entity<ChildBase>()
.HasDiscriminator(x => x.Discriminator)
.HasValue<GoodChild>(ParentChildDiscriminator.Good)
.HasValue<BadChild>(ParentChildDiscriminator.Bad);
}
}
} |
The derived children are discovered by conventions and added to model (no TPH there yet). But when we add |
Good point on the SaveChanges() being mostly (if not entirely) unnecessary. But, see my question #15581 - I got a DbUpdateException after moving past this issue. As discussed by another developer in #14890 , it would probably be ideal to be able to call I have more TPH bugs to file, but I have a deadline to get a project done this week and have to optimize what I think are the most valuable issues to file. For example, TPH gets confused with auto-identities. This would not be uncovered with just |
Just my 2 cents - may not be helpful, but having a background as a SQL person who learned C# out of necessity, I think of it as a full outer join and filtering the EF Model against the SQL Model ( Sorry if this is philosophical drivel, but I believe "perspective is worth 80 IQ points". |
@jzabroski - My comment was note to the implementer of bug fix. I took a dive in codebase to make sense of stacktrace you posted so those are my findings from brief inspection. |
I understand - thanks for your speedy answers! |
Repro'd in 3.0.0-preview4 |
Repro'd again in 4f1f019 (close to preview7). Ready-to-run repro for QueryBugTests: #region Bug15553
[ConditionalFact]
public virtual void Discriminator_name_must_be_unique_across_TPH_hierarchies()
{
using (CreateDatabase15553())
{
}
}
private SqlServerTestStore CreateDatabase15553()
=> CreateTestStore(
() => new MyContext15553(_options),
context => ClearLog() );
public class MyContext15553 : DbContext
{
public MyContext15553(DbContextOptions options) : base(options) {}
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<ParentBase15553>()
.HasDiscriminator(x => x.Discriminator)
.HasValue<GoodParent15553>(1)
.HasValue<BadParent15553>(2);
modelBuilder.Entity<ChildBase15553>()
.HasDiscriminator(x => x.Discriminator)
.HasValue<GoodChild15553>(3)
.HasValue<BadChild15553>(4);
}
}
private abstract class ParentBase15553
{
public int Id { get; set; }
public int Discriminator { get; protected set; }
}
private class GoodParent15553 : ParentBase15553 {}
private class BadParent15553 : ParentBase15553
{
// Commenting out the following makes the test pass
public BadChild15553 Child { get; set; }
}
private abstract class ChildBase15553
{
public int Id { get; set; }
public int Discriminator { get; protected set; }
}
private class GoodChild15553 : ChildBase15553 {}
private class BadChild15553 : ChildBase15553 {}
#endregion Bug15553 |
Note from servicing meeting: we will fix the simple case here (See #18769) in 5.0 and if the risk is low then port to 3.1.x. Putting in 3.1.x for now so we track it as a potential patch. |
Using #18769 to track the servicing fix. |
Neither repro fails with 3.1.1 But the discriminator still doesn't use the value converter correctly, see #19650 |
Describe what is not working as expected.
Two entities in two separate table-per-hierarchy mappings cannot have the same discriminator name.
Note: In the supplied GitHub project, I have drawn an entity model which provides humanistic view on why this is important and useful.
Workaround (I Hope)
Manually creating unique property names for each table-per-hierarchy mapping works, but is less-than-ideal and worrisome (what have I not uncovered).
If you are seeing an exception, include the full exceptions details (message and stack trace).
Steps to reproduce
dotnet test --filter ProgramOneTphToOneTph.One_TablePerHierarchy_To_One_TablePerHierarchy
dotnet test --filter
does not filter xUnit.net Theory Data. Due to above concerns about thread safety, it is recommended to run the tests in Visual Studio where you can manually select individial Theory Data as a test case, and run each Theory Data one-by-one. (I have confirmed both ReSharper and Visual Studio Test Explorer support manually selecting Theory Data items and running one-by-one.)Further technical details
EF Core version: 2.2.4
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2017 15.9.11
Target Framework: netcoreapp2.2
Target Architecture: Any CPU
The text was updated successfully, but these errors were encountered: