simpleidserver / EFCore.Cassandra

Entity Framework Core provider for Cassandra
Apache License 2.0
35 stars 16 forks source link

Shadow Foreign Key issue with UDTs #20

Closed Delta-38 closed 3 years ago

Delta-38 commented 3 years ago

Hi,

while testing branch 2.0.5 I ran into a strange issue with Migrations. (Running migrations generated on release 2.0.4)

I have a mapped entity (We'll call it BOB) which also contains a set<frozen> (we'll call that Fred) IEnumberable property. Fred is mapped and has "HasNoKey()" specified in the modelbuilder.

When running this on 2.0.5, EF Core attempts to add Shadow Foreign Keys to Fred to match the primary keys of Bob, throwing errors during startup. All attempts at generating new migrations fail unless I remove the DbSet from my context class.

The short version is that when a DbSet includes a set<frozen> and with the udt being specified as "HasNoKey()" in the modelbuilder, EF attempts to create shadow foreign keys to match to the containing entity keys, which throws exceptions during startup and migration as there are no matching properties in the models. The errors are thrown at EFCassandraDbConnection:79.

I am generating migrations code first and running them by calling MigrateAsync on the DbContext. Apart from declaring the mapped tables as DbSets (ex: public DbSet Things) and using config classes (like: class ThingsConfig : IEntityTypeConfiguration ) invoked in the OnModelCreating method of the DbContext.

Thanks in advance

simpleidserver commented 3 years ago

Hello,

I tried to reproduce the issue on my local machine without success. You'll find in attachment my testing project, can-you please use it to reproduce the error ?

KR EFCore.Cassandra.Samples.zip

Delta-38 commented 3 years ago

Hello,

thanks for sharing the simplified example, it allowed me to track down the issue in my code. The behavior emerges if there is a slight mismatch as in: builder.Property(c => c.Freds).HasColumnName("freds").HasColumnType<IList>("set<frozen>"); in this case, foreign keys get added during the creation of the migrations and of course fail when running the migration at runtime. Once I corrected it to IEnumerable on both ends and recreated the migrations that was sorted.

As an aside, is set< > supported? I noticed that when requiring column type as set<frozen> when the migration is run list<frozen> is what get created on the database.

I am sending back both the example with the error (hoping it can be useful) and the example where set gets created as list.

foreignkeyadded.zip efcore.cassandra set to list.zip

Delta-38 commented 3 years ago

Regarding the Collection statement forcing "list" type issue, it seems rooted in the way the statement is built, at CassandraMigrationSqlGenerator: 240 ish, since it assumes the collection type to be list, perhaps a simple change in this fashion could do the trick (not very stylish I know):

 var collection = GetCollectionType(columnType);
 columnType = $"{collection}<FROZEN<{tn}>>";
 ....                   
 private string GetCollectionType(string columnType)
 {
    var parts = columnType.Split("<");
    if (parts.Count() > 1)
    {
        string type = parts.FirstOrDefault();
        string[] supportedCollections = new string[] { "set", "map", "list" };
        if (supportedCollections.Contains(type.ToLower()))
        {
            return type;
        }
    }
    return "list";  
 }
simpleidserver commented 3 years ago

Hello,

I made some changes in the branch "release/2.0.5" to remove shadow properties from migration scripts. For the other concern, I need some times to make some investigations :)

Delta-38 commented 3 years ago

Thank you very much, I'll look at them right away. Of course, no problem, I didn't mean to rush you :)

simpleidserver commented 3 years ago

Hello,

I made some modifications in the branch "release/2.0.5" to support "list", "set" etc... Can-you please check ?

KR

Delta-38 commented 3 years ago

Hi, sorry I couldn't get back to you sooner. I've tested the changes you've made and they're working great.