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

Updating JSON column when query tracking is disabled throws #33862

Closed
vladislav-karamfilov opened this issue May 31, 2024 · 11 comments
Closed

Updating JSON column when query tracking is disabled throws #33862

vladislav-karamfilov opened this issue May 31, 2024 · 11 comments

Comments

@vladislav-karamfilov
Copy link

vladislav-karamfilov commented May 31, 2024

File a bug

Setting a new value to a JSON-mapped column property (a non-collection or a collection one) when query tracking is disabled fails with InvalidOperationException. Enabling query tracking with optionsBuilder.UseQueryTrackingBehavior(QueryTrackingBehavior.TrackAll) fixes the issue but it is not an option for the project I'm working on.

PS All is working well when we have primitive collections like List<string> in the entity instead of JSON object(s).

Include your code

using Microsoft.EntityFrameworkCore;

await EnsureThingsInDbAsync();

// These method calls throw!!!
await UpdateThingAsync(isManualUpdate: false, updateSinglePart: false);
await UpdateThingAsync(isManualUpdate: false, updateSinglePart: true);

// These method calls doesn't throw but the update is not performed (the value is not updated in DB)!!!
await UpdateThingAsync(isManualUpdate: true, updateSinglePart: false);
await UpdateThingAsync(isManualUpdate: true, updateSinglePart: true);

static async Task UpdateThingAsync(bool isManualUpdate, bool updateSinglePart)
{
    using var db = new AppDbContext();
    var thing = await db.Things.FirstOrDefaultAsync();

    if (updateSinglePart)
    {
        thing!.SinglePart = new Part { Name = "updated single part " + Random.Shared.Next() };
    }
    else
    {
        thing!.Parts = [new Part { Name = "updated part " + Random.Shared.Next() }];
    }

    if (isManualUpdate)
    {
        var entry = db.Entry(thing);
        if (entry.State == EntityState.Detached)
        {
            db.Things.Attach(thing);
        }

        entry.State = EntityState.Modified;
    }
    else
    {
        db.Things.Update(thing);
    }

    await db.SaveChangesAsync();
}

static async Task EnsureThingsInDbAsync()
{
    using var db = new AppDbContext();
    await db.Database.EnsureCreatedAsync();

    if (await db.Things.AnyAsync())
    {
        return;
    }

    db.Things.Add(new Thing
    {
        Parts = [new Part { Name = "part " + Random.Shared.Next() }],
        SinglePart = new Part { Name = "single part " + Random.Shared.Next() },
    });

    await db.SaveChangesAsync();
}

public class AppDbContext : DbContext
{
    public DbSet<Thing> Things { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
        => modelBuilder.Entity<Thing>()
            .OwnsOne(x => x.SinglePart, b => b.ToJson())
            .OwnsMany(x => x.Parts, b => b.ToJson());

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        base.OnConfiguring(optionsBuilder);

        optionsBuilder.UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=ef-json-cols");

        optionsBuilder.UseQueryTrackingBehavior(QueryTrackingBehavior.NoTrackingWithIdentityResolution);
        // optionsBuilder.UseQueryTrackingBehavior(QueryTrackingBehavior.NoTracking); // Doesn't work too!!!
    }
}

public class Thing
{
    public int Id { get; set; }

    public List<Part> Parts { get; set; } = [];

    public Part? SinglePart { get; set; }
}

public class Part
{
    public required string Name { get; set; }
}

Include stack traces

Unhandled exception. System.InvalidOperationException: The value of shadow key property 'Thing.Parts#Part (Part).Id' is unknown when attempting to save changes. This is because shadow property values cannot be preserved when the entity is not being tracked. Consider adding the property to the entity's .NET type. See https://aka.ms/efcore-docs-owned-collections for more information.
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.<PrepareToSave>g__CheckForUnknownKey|111_0(IProperty property, <>c__DisplayClass111_0&)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.PrepareToSave()
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.GetEntriesToSave(Boolean cascadeChanges)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Program.<<Main>$>g__UpdateThingAsync|0_0(Boolean isManualUpdate, Boolean updateSinglePart) in C:\Users\Vladislav\source\repos\ConsoleApp9\Program.cs:line 42
   at Program.<Main>$(String[] args) in C:\Users\Vladislav\source\repos\ConsoleApp9\Program.cs:line 8
   at Program.<Main>(String[] args)

Include provider and version information

EF Core version: 8.0.6
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 8.0
Operating system: Windows 11
IDE: Visual Studio 2022 17.10.1

@vladislav-karamfilov vladislav-karamfilov changed the title Updating JSON column when query tracking is disabled fails Updating JSON column when query tracking is disabled throws Jun 1, 2024
@maumar maumar self-assigned this Jun 3, 2024
@vladislav-karamfilov
Copy link
Author

vladislav-karamfilov commented Jun 7, 2024

@maumar , I see that you have self-assigned yourself a couple of days ago. Have you had the chance to investigate this bug in details?

I have just tested on .NET 7 and it still fails. I hope that you'll fix it for some EF Core 8 patch version or at worst for the .NET 9 release.

@maumar
Copy link
Contributor

maumar commented Jun 7, 2024

@vladislav-karamfilov no, I didn't investigate this issue yet. JSON is generally my area to look after, so I just assigned myself for now, so that other team members don't need to read through the issue. I will post my findings here when I get to it, but it may take some time - we have a big backlog of uninvestigated issues at the moment.

@maumar
Copy link
Contributor

maumar commented Jul 15, 2024

workaround is to do manual update by adding the entry and setting the state to unchanged. This forces generation of all the shadow values.

            var entry = db.Entry(thing);
            if (entry.State == EntityState.Detached)
            {
                db.Things.Add(thing);
            }

            entry.State = EntityState.Unchanged;

@maumar maumar added this to the Backlog milestone Jul 15, 2024
@vladislav-karamfilov
Copy link
Author

@maumar , the project I'm working on (where I found this issue) is massive and we use the approach with manual update from the issue description (because of various reasons not related to this issue):

var entry = db.Entry(thing);
if (entry.State == EntityState.Detached)
{
    db.Things.Attach(thing);
}

entry.State = EntityState.Modified;

What are the consequences of using the new suggested manual update approach? What behavioral changes will we observe? Is there a way to narrow down the entry objects which have JSON columns (skipping the ones with primitive collections) so we can apply the new update only on a subset of all updates that we do in the app?

PS I see that you have put this in Backlog milestone. Does that mean that you don't plan to fix this for EF Core 9? And if the answer is "Yes", is there any specific reason (it seems like 2 major features of EF Core cannot cooperate because of a bug and thus I would consider this as important bug to fix)?

@maumar
Copy link
Contributor

maumar commented Jul 22, 2024

looping in @ajcvickers and @AndriySvyryd, who are the domain experts, wrt consequences and subtleties of this approach vs the original one.

As to your other question, Backlog milestone unfortunately means we are not planning to fix this bug in 9. Reason being, there is not much development time left before we close the release, the fix is likely non-trivial/risky, and there is a reasonable workaround. All those factors push this issue down in the priority for us, compared to other items the team is working on.

@ajcvickers ajcvickers removed this from the Backlog milestone Aug 12, 2024
@ajcvickers
Copy link
Member

This seems by-design to me. Marking for discussion by team.

@vladislav-karamfilov
Copy link
Author

@ajcvickers , I'm not sure I understand. Aren't these major EF features (no query tracking + JSON columns) supposed to work together? Our team's expectation is to have this scenario working out of the box just like the query tracking ON + JSON columns are working nicely together.

@ajcvickers
Copy link
Member

ajcvickers commented Aug 12, 2024

@vladislav-karamfilov If you're explicitly tracking entities and using shadow key properties, then you need to make sure that these shadow properties have been set appropriately. Alternatively, don't use shadow keys, which means that all the values will travel with the detached entity.

@vladislav-karamfilov
Copy link
Author

@ajcvickers , we are not explicitly tracking entities and we are not doing anything specific with shadow properties and everything is working well for us. But recently we saw opportunity to use JSON columns for some of our entity props and we hit this issue. We don't know how to overcome it and IMHO it seems like something that should work perfectly out of the box. I hope you are going to fix it as soon as possible so we can leverage this EF feature soon.

PS If you have any suggestions about a workaround, please share it with us. How will the one suggested @maumar affect our EF usage having in mind how we are using it?

@ajcvickers
Copy link
Member

@vladislav-karamfilov Unfortunately, using owned entities results in this kind of issue. They are not a great match for JSON, since they are still entity types, just with hidden identity. Please vote for Add relational JSON mapping support for complex types, which should make this much better.

we are not explicitly tracking entities

I mean calling Attach or Update on entities queried by a different context instance. See Explicitly Tracking Entities.

@ajcvickers ajcvickers added this to the Backlog milestone Aug 14, 2024
@ajcvickers ajcvickers removed this from the Backlog milestone Aug 14, 2024
@vladislav-karamfilov
Copy link
Author

@vladislav-karamfilov Unfortunately, using owned entities results in this kind of issue. They are not a great match for JSON, since they are still entity types, just with hidden identity. Please vote for Add relational JSON mapping support for complex types, which should make this much better.

I have already upvoted this issue and my colleagues will upvote too. I see that this is the most requested feature in the open issues list. Is there any chance for this issue to make it for the .NET 9 release?

we are not explicitly tracking entities

I mean calling Attach or Update on entities queried by a different context instance. See Explicitly Tracking Entities.

I have read this article in the past but I will read it again to search for more info about this workaround. Thanks, @ajcvickers!

@maumar maumar closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2024
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

3 participants