EFCore.BulkExtensions icon indicating copy to clipboard operation
EFCore.BulkExtensions copied to clipboard

Navigation Property not saving with BulkInsert

Open nepdev opened this issue 2 years ago • 9 comments

I have used EFCore.BulkExtensions for about a year (.Net Core 3.1, SQL Server 2019) and in my experience, navigation properties would always automatically be populated, even if there is no explicitly defined reference column.

For example, following simple classes:

public class Customer
{
    public int ID { get; set; }
    public String Name { get; set; }
    public Cont Continent { get; set; }        
}

public class Cont
{
    public int ID { get; set; }
    public String Name { get; set; }
}

When creating a List<Customer> with properly populated "Cont" objects (with proper ID as it exists in the database) then I always had success that the customer table in the database had the implicitly created "ContinentID" column properly populated with the ID of the "Cont" object.

I upgraded to 3.6.1 and it stopped working: the ContinentID prop is always NULL for every single customer entity. The version I used before was 3.1 or thereabouts. However, downgrading back to 3.1 does not help. Tried to up- and downgrade to all sorts of version with no luck at all.

On low versions like 3.1.6 I get a different error

    The given value 'Customer' of type String from the data source cannot be converted to type int for Column 2 [ContinentID] Row 1.
    ----> System.FormatException : Failed to convert parameter value from a String to a Int32.
    ----> System.FormatException : Input string was not in a correct format.

but does not work.

Tried to use "IncludeGraph" but conceptually, that's the other way around really - it would try to insert "Cont" objects in above scenario if not existing yet. At least that's my understanding.

I found one way around it - adding the reference column explicitly in every entity (e.g. int property ContinentID in above example in the Customer class), and manually, in my code, populate the reference prop. Then it does work properly.

But do I HAVE to go that route? I have a lot of entities.

Was there some sort of fundamental change, or a change in a related / dependent package which makes this no longer work? What is my route to handle this problem? Will an upgrade to EFCore 5 and BulkExtensions 5.x help?

I do not necessarily want to upgrade EFCore 5 since it's not LTS.

nepdev avatar Jul 08 '21 20:07 nepdev

Navigation prop. support was added in 3.2.1 https://github.com/borisdj/EFCore.BulkExtensions/issues/139 You can check it on test ShadowFKPropertiesTest with ItemLink. If still having issues try to alter that test to reproduce it.

borisdj avatar Jul 09 '21 11:07 borisdj

Thanks.

Noticed that my code works perfectly with version 3.2.5. Cannot find test ShadowFKPropertiesTest in the project.

The only test I can find regarding shadow properties has nothing to do with foreign keys, and only tests that we can manually set them and they are saved to the database. As mentioned, I know that this works.

What does not work is the automatic value assignment in case the shadow property is an automatic prop created for a foreign key relation. It does seem to work in some of the versions of the package.

Checked various package versions, here the result

3.2.5 works, flag "EnableShadowProperties" not yet supported 3.3.0 works, flag "EnableShadowProperties" not yet supported 3.3.5 works both with EnableShadowProperties = false / true 3.3.6 works with EnableShadowProperties = false, gives duplicate column error with EnableShadowProperties = true 3.3.7 works with EnableShadowProperties = false, gives duplicate column error with EnableShadowProperties = true 3.3.8 works with EnableShadowProperties = false, gives duplicate column error with EnableShadowProperties = true 3.3.9 does not work, neither with EnableShadowProperties = true, or false 3.4.0 does not work, neither with EnableShadowProperties = true, or false ... 3.6.1 does not work, neither with EnableShadowProperties = true, or false

The duplicate column error (package versions 3.3.6 - 3.3.8) does not occur on every entity. Could not determine why it happens on some. Simple entities with a single foreign key relation don't seem to have a problem.

nepdev avatar Jul 09 '21 17:07 nepdev

I was referring to this Test: https://github.com/borisdj/EFCore.BulkExtensions/blob/ebc6d2786fd9e24f4820524330558cb00c3f19d4/EFCore.BulkExtensions.Tests/EFCoreBulkTestAtypical.cs#L432 or in v3 branch: https://github.com/borisdj/EFCore.BulkExtensions/blob/53609bb6151bc0884692e68d513cfa773522074b/EFCore.BulkExtensions.Tests/EFCoreBulkTestAtypical.cs#L340

If you do not plan upgrading to v5 until 6 is fully released, which is soon, then keep it on v3.2.5 if it works fine.

borisdj avatar Jul 09 '21 17:07 borisdj

I can keep it on 3.2.5

Did some more checking on this. I am noting what I found, in case it helps. From what I have seen the error might also be occuring in the 5 branch, undetected.

The test ShadowFKPropertiesTest is not so very good.

Occasionally the BulkInsert command inserts no records at all (silently with no error message), but there is no Assert for that in the test. You do a BulkRead(entities), and this does not change "entities" if no data is returned, which means, in such a case, you may end up "testing" the entities collection you just created in memory, not what the database holds.

And even if BulkRead() would return an empty collection, the subsequent loop would not run and the test passes, even though nothing was inserted at all.

(The "context.BulkDelete(context.ItemLinks.ToList());" at the end of the tests makes that also easy to miss in the database itself.)

I would prefer to just obtain the collection with dbContext.ItemLinks.ToList() to be sure, and do an assert to make sure some records actually got inserted.

So instead of the following

        if (dbServer == DbServer.SqlServer)
        {
            context.BulkRead(entities);
            foreach (var entity in entities)
            {
                Assert.NotNull(entity.Item);
            }
        }

the code should be like this

        if (dbServer == DbServer.SqlServer)
        {
            List<ItemLink> links = context.ItemLinks.ToList();
            Assert.True(links.Count() > 0, "ItemLink row count");
            foreach (var link in links)
            {
                Assert.NotNull(link.Item);
            }
        }

Now this "not inserting any records" does not happen for every entity type. Tested with 4 different classes, with integer and string type foreign keys, and here is what I see:

  • some entity types get no records inserted (ItemLink entity, for example)
  • some entity types get the records inserted but foreign key property is NULL
  • none work properly

All of the above data is for version 3.6.1. If we are using version 3.3.5, every test passes for the entities we use (there does seem some which don't work), so something seems broken after 3.3.5.

nepdev avatar Jul 10 '21 19:07 nepdev

I have confirmed that FK remains Null even on current version. The issue being current value is here null, not sure why. https://github.com/borisdj/EFCore.BulkExtensions/blob/ebc6d2786fd9e24f4820524330558cb00c3f19d4/EFCore.BulkExtensions/SQLAdapters/SQLServer/SqlServerAdapter.cs#L688

borisdj avatar Jul 12 '21 12:07 borisdj

Hello there,

This seems to still be an issue on v5.4.0. I used the "IncludeGraph" option link child POCOs to their parent POCO, but the FK in the children pointing to their parent is always null. It is a but special as the children are of the same type as the parent. So they are FKs pointing to the same table (see examples below).

By activating "TrackingEntities", I saw that the ParentIDs are tracked in the children, but not set in DB. My current workaround is to do an BulkUpdate with the tracked FKs, but it is not ideal.

Any update on this?

Here is an example of the table that is used:

public class Fund 
{
   public int Id { get; set; }
   public int? UmbrellaFundId { get; set; }
   public string OfficialName { get; set; }
   public DateTime DataIn { get; set; }
   public DateTime DataOut { get; set; }

   public virtual Fund UmbrellaFund { get; set; } // this nav prop is set before bulk insert to be used to create to link to the parent
   public virtual ICollection<Fund> SubFunds { get; set; } // this is the inverse of the nav prop from before, which is not used and left empty
}
create table [Master].[Fund]
(
    [Id] int identity (1, 1) not null,
    [UmbrellaFundId] int null, -- this is the FK which is null after bulk insert, but tracked when tracking is enabled
    [OfficialName] nvarchar(256) not null,
    [DataIn] datetime2 generated always as row start not null,
    [DataOut] datetime2 generated always as row end   not null,
    period for system_time (DataIn, DataOut),
    constraint [PK_Fund] primary key clustered ([Id] asc),
    constraint [FK_Fund_UmbrellaFund] foreign key ([UmbrellaFundId]) references [Master].[Fund] ([Id]), -- pointing to itself
) with (system_versioning = on (HISTORY_TABLE = [History].[FundHistory]));

EDIT: here is the method I use to Insert&Update (simplified):

public async Task<bool> BulkInsertAsync(IList<Fund> funds)
{
    try
    {
        await using var transaction = await Context.Database.BeginTransactionAsync();
        await Context.BulkInsertAsync(funds, new BulkConfig
        {
            SetOutputIdentity = true,
            IncludeGraph = true,
            TrackingEntities = true
        });

        var subFunds = funds.Where(f => f.UmbrellaFundId is not null).ToList();

        await Context.BulkUpdateAsync(subFunds);
        await transaction.CommitAsync();
        return true;
    }
    catch (Exception e)
    {
        return false;
    }
}

pjominet avatar Oct 19 '21 10:10 pjominet

I see the test ShadowFKPropertiesTest only covers SQL server. Is Postgres unsupported for shadow FKs? If so is there any work around other than adding the ID field directly into the parent entity? I am trying to attach existing objects as FKs to entities to insert/update. The object structure is the same as napdevs original example.

zmerdev avatar Apr 07 '22 23:04 zmerdev

I'm having trouble with this for current version, EFC 6, .NET 6

using

        {
            SetOutputIdentity = true,
            IncludeGraph = true,
            TrackingEntities = true
        }

Bulk operations try to insert 0 for the key instead of observing auto generated values If I do not supply a config, it inserts correctly but does not emit identity values

if I remove IncludeGraph it works but it then fails if I bulk insert things that refer to them as navigation properties keys are now 0 making the bulk insert of these objects fail because EF thinks all these 0 IDs are new entities.

Eventually, however, on a loop of bulk inserts per data sets, it'll fail for a bulk insert on the parent entity for duplicate ekys

Prinsn avatar Sep 29 '22 21:09 Prinsn