Pomelo.entityframeworkcore.mysql: JsonObject property setters do not track changes in array elements

[haruby511]

Steps to reproduce

Copy paste the IceCreames example.
Add and change Program.Main() to below.

using (var context = new Context())
{
    context.Database.EnsureDeleted();
    context.Database.EnsureCreated();

    context.IceCreams.AddRange(
        new IceCream
        {
            Name = "Vanilla",
            Energy = new Energy
            {
                Kilojoules = 866.0,
                Kilocalories = 207.0,
            },
            Comments = new[]
            {
                    "First!",
                    "Delicios!"
            },
        },
        new IceCream
        {
            Name = "Chocolate",
            Energy = new Energy
            {
                Kilojoules = 904.0,
                Kilocalories = 216.0,
            },
            Comments = new[]
            {
                    "My husband linkes this one a lot."
            },
        });

    context.SaveChanges();
}

using (var context = new Context())
{
    IceCream iceCream = context.IceCreams.Where(e => e.IceCreamId == 1).First();
    //iceCream.Comments.Object[0] = "Test update.";
    //iceCream.Comments.Object = new string[] { "Test update.", "Second line." };
    //iceCream.Comments.Json = "[ 'Test update.', 'Second line.' ]";
    iceCream.Comments = new string[] { "Test update.", "Second line." };
    iceCream.Comments = "[ 'Test update.', 'Second line.' ]";
    context.SaveChanges();
}

using (var context = new Context())
{
    var result = context.IceCreams
        .OrderBy(e => e.IceCreamId)
        .ToList();

    Debug.Assert(result.Count == 2);

    Debug.Assert(result[0].Name == "Vanilla");
    Debug.Assert(result[0].Energy.Object.Kilojoules == 866.0);
    Debug.Assert(result[0].Comments.Object.Length == 2);
    Debug.Assert(result[0].Comments.Object[0] == "Test update.");
}

The issue

Assertion failed with below commented cases.

//iceCream.Comments.Object[0] = "Test update.";
//iceCream.Comments.Object = new string[] { "Test update.", "Second line." };
//iceCream.Comments.Json = "[ 'Test update.', 'Second line.' ]";
iceCream.Comments = new string[] { "Test update.", "Second line." };
iceCream.Comments = "[ 'Test update.', 'Second line.' ]";

Only assertion succeed with implicit operators(last two ones).

I can understand first one fails. It is almost impossible to track changes and other systems also do not consider such like things.

But it is confusing second and third one fail which are accessing property setters. I think, removing the setters or implement as same with the implicit operators is better.

Further technical details

MySQL version: MariaDB 10.3
Operating system: Windows 10
Pomelo.EntityFrameworkCore.MySql version: 3.12
Microsoft.AspNetCore.App version: .NET Core 3.1 Console Application

[lauxjpn]

Yeah, the JsonObject<T> implementation isn't clever enough to handle changes inside of arrays. You need to replace the whole JsonObject<T>. Because of operator overloading, you can also just replace the whole array:

```c#
iceCream.Comments.Object[0] = "Test update.";
iceCream.Comments = (string[]) iceCream.Comments.Object.Clone();

Alternatively, you could introduce some extension method:

```c#
public static class JsonObjectExtensions
{
    public static JsonObject<T[]> SetAt<T>(this JsonObject<T[]> source, int index, T value)
    {
        var clone = source.Object.ToArray();
        clone[index] = value;
        return new JsonObject<T[]>(clone);
    }
}

That could be used like this:

c# iceCream.Comments = iceCream.Comments.SetAt(0, "Test update.");

I don't think there is a lot of merit for us to officially extend JsonObject<T>, because I basically finished implementing the full JSON support, so JsonObject<T> will become obsolete shortly.

[haruby511]

@lauxjpn Would you provide me a example for JSON example without the JsonObject<T>? As you know, there is not info about JSON features in the docs.

[lauxjpn]

@haruby511 This has not been published yet, but will be soon in the nightly builds. See #1102 for updates on this.

[lauxjpn]

This has not been fixed yet in the new JSON support implementations.

@roji I haven't checked yet, whether this is also an issue in Npgsql, but it likely could be. The issue is that changes to elements of an array, that is itself the root of a JSON POCO or just a property of a JSON POCO, are not being detected. Is this intended or unexpected?

Sample code that fails, with the array as the root

```c#
using System.Diagnostics;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Pomelo.EntityFrameworkCore.MySql.Storage;

namespace IssueConsoleTemplate
{
public class IceCream
{
public int IceCreamId { get; set; }
public string Name { get; set; }
public Energy Energy { get; set; }
public string[] Comments { get; set; }
}

// This is a JSON POCO:
public class Energy
{
    public double Kilojoules { get; set; }
    public double Kilocalories { get; set; }
}

public class Context : DbContext
{
    public virtual DbSet<IceCream> IceCreams { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseMySql("server=127.0.0.1;port=3306;user=root;password=;database=Issue1140_02",
                b => b
                    .ServerVersion(new ServerVersion("8.0.20-mysql"))
                    .UseMicrosoftJson())
            .UseLoggerFactory(LoggerFactory.Create(b => b
                .AddConsole()
                .AddFilter(level => level >= LogLevel.Information)))
            .EnableSensitiveDataLogging()
            .EnableDetailedErrors();
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<IceCream>(
            entity =>
            {
                entity.Property(e => e.Energy)
                    .HasColumnType("json");
                entity.Property(e => e.Comments)
                    .HasColumnType("json");
            });
    }
}

internal static class Program
{
    private static void Main()
    {
        using (var context = new Context())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.IceCreams.AddRange(
                new IceCream
                {
                    Name = "Vanilla",
                    Energy = new Energy
                    {
                        Kilojoules = 866.0,
                        Kilocalories = 207.0,
                    },
                    Comments = new[]
                    {
                        "First!",
                        "Delicios!"
                    },
                },
                new IceCream
                {
                    Name = "Chocolate",
                    Energy = new Energy
                    {
                        Kilojoules = 904.0,
                        Kilocalories = 216.0,
                    },
                    Comments = new[]
                    {
                        "My husband linkes this one a lot."
                    },
                });

            context.SaveChanges();
        }

        using (var context = new Context())
        {
            var iceCream = context.IceCreams.First(e => e.IceCreamId == 1);
            iceCream.Comments[0] = "Test update.";
            context.SaveChanges(); // <-- does not detect the change.
        }

        using (var context = new Context())
        {
            var result = context.IceCreams
                .OrderBy(e => e.IceCreamId)
                .ToList();

            Debug.Assert(result.Count == 2);
            Debug.Assert(result[0].Name == "Vanilla");
            Debug.Assert(result[0].Energy.Kilojoules == 866.0);
            Debug.Assert(result[0].Comments.Length == 2);
            Debug.Assert(result[0].Comments[0] == "Test update."); // <-- fails
        }
    }
}

}

</details>
<details>
<summary>Sample code that fails, with the array as a property of a POCO</summary>

```c#
using System.Diagnostics;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Pomelo.EntityFrameworkCore.MySql.Storage;

namespace IssueConsoleTemplate
{
    public class IceCream
    {
        public int IceCreamId { get; set; }
        public string Name { get; set; }
        public Energy Energy { get; set; }
    }

    // This is a JSON POCO:
    public class Energy
    {
        public double Kilojoules { get; set; }
        public double Kilocalories { get; set; }
        public string[] Comments { get; set; }
    }

    public class Context : DbContext
    {
        public virtual DbSet<IceCream> IceCreams { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
                .UseMySql("server=127.0.0.1;port=3306;user=root;password=;database=Issue1140_03",
                    b => b
                        .ServerVersion(new ServerVersion("8.0.20-mysql"))
                        .UseMicrosoftJson())
                .UseLoggerFactory(LoggerFactory.Create(b => b
                    .AddConsole()
                    .AddFilter(level => level >= LogLevel.Information)))
                .EnableSensitiveDataLogging()
                .EnableDetailedErrors();
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<IceCream>(
                entity =>
                {
                    entity.Property(e => e.Energy)
                        .HasColumnType("json");
                });
        }
    }

    internal static class Program
    {
        private static void Main()
        {
            using (var context = new Context())
            {
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();

                context.IceCreams.AddRange(
                    new IceCream
                    {
                        Name = "Vanilla",
                        Energy = new Energy
                        {
                            Kilojoules = 866.0,
                            Kilocalories = 207.0,
                            Comments = new[]
                            {
                                "It's not that much!",
                                "We could improve on that anyway!"
                            },
                        },
                    },
                    new IceCream
                    {
                        Name = "Chocolate",
                        Energy = new Energy
                        {
                            Kilojoules = 904.0,
                            Kilocalories = 216.0,
                            Comments = new[]
                            {
                                "I don't care about this at all!"
                            },
                        },
                    });

                context.SaveChanges();
            }

            using (var context = new Context())
            {
                var iceCream = context.IceCreams.First(e => e.IceCreamId == 1);
                iceCream.Energy.Comments[0] = "Test update.";
                context.SaveChanges(); // <-- does not detect the change.
            }

            using (var context = new Context())
            {
                var result = context.IceCreams
                    .OrderBy(e => e.IceCreamId)
                    .ToList();

                Debug.Assert(result.Count == 2);
                Debug.Assert(result[0].Name == "Vanilla");
                Debug.Assert(result[0].Energy.Kilojoules == 866.0);
                Debug.Assert(result[0].Energy.Comments.Length == 2);
                Debug.Assert(result[0].Energy.Comments[0] == "Test update."); // <-- fails
            }
        }
    }
}

This actually applies to changes of any property of a JSON POCO, which will not get detected (which makes sense, because the POCO is not an entity of the model itself, so the change tracker will not iterate over its properties):

Sample code that fails, with any JSON POCO property changed

```c#
using System.Diagnostics;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Pomelo.EntityFrameworkCore.MySql.Storage;

namespace IssueConsoleTemplate
{
public class IceCream
{
public int IceCreamId { get; set; }
public string Name { get; set; }
public Energy Energy { get; set; }
}

// This is a JSON POCO:
public class Energy
{
    public string CategoryName { get; set; } 
    public double Kilojoules { get; set; }
    public double Kilocalories { get; set; }
    public string[] Comments { get; set; }
}

public class Context : DbContext
{
    public virtual DbSet<IceCream> IceCreams { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseMySql("server=127.0.0.1;port=3308;user=root;password=;database=Issue1140_04",
                b => b
                    .ServerVersion(new ServerVersion("8.0.20-mysql"))
                    .UseMicrosoftJson())
            .UseLoggerFactory(LoggerFactory.Create(b => b
                .AddConsole()
                .AddFilter(level => level >= LogLevel.Information)))
            .EnableSensitiveDataLogging()
            .EnableDetailedErrors();
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<IceCream>(
            entity =>
            {
                entity.Property(e => e.Energy)
                    .HasColumnType("json");
            });
    }
}

internal static class Program
{
    private static void Main()
    {
        using (var context = new Context())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.IceCreams.AddRange(
                new IceCream
                {
                    Name = "Vanilla",
                    Energy = new Energy
                    {
                        CategoryName = "Medium",
                        Kilojoules = 866.0,
                        Kilocalories = 207.0,
                        Comments = new[]
                        {
                            "It's not that much!",
                            "We could improve on that anyway!"
                        },
                    },
                },
                new IceCream
                {
                    Name = "Chocolate",
                    Energy = new Energy
                    {
                        CategoryName = "High",
                        Kilojoules = 904.0,
                        Kilocalories = 216.0,
                        Comments = new[]
                        {
                            "I don't care about this at all!"
                        },
                    },
                });

            context.SaveChanges();
        }

        using (var context = new Context())
        {
            var iceCream = context.IceCreams.First(e => e.IceCreamId == 1);
            iceCream.Energy.CategoryName = "Delicious";
            context.SaveChanges(); // <-- does not detect the change
        }

        using (var context = new Context())
        {
            var result = context.IceCreams
                .OrderBy(e => e.IceCreamId)
                .ToList();

            Debug.Assert(result.Count == 2);
            Debug.Assert(result[0].Name == "Vanilla");
            Debug.Assert(result[0].Energy.CategoryName == "Delicious"); // <-- fails
            Debug.Assert(result[0].Energy.Kilojoules == 866.0);
            Debug.Assert(result[0].Energy.Comments.Length == 2);
        }
    }
}

}
```

Taking a look at ChangeDetector.LocalDetectChanges(), it should be enough for the JSON type mapping to provide a value comparer, that does its own JSON POCO traversal for comparison (or maybe simpler, just compare the old and new serialized result, if possible).

This is being confirmed by Value Comparers.

[roji]

Yeah, in the Npgsql JSON support, automatic change tracking does not work because there is no value comparer - this is by-design. It's a good idea to be very careful with comparers for arbitrarily-complex/long types, such as JSON or arrays, because the perf impact of both snapshotting the entire JSON tree (when loading), and traversing for comparison (when detecting changes upon save) can be really significant. For now I recommend to users to flag the change manually (see https://github.com/npgsql/efcore.pg/issues/1228#issuecomment-581379474 for example).

Cross-database JSON support is likely to be one of my focuses for EF Core 6.0 (https://github.com/dotnet/efcore/issues/4021), so we'll likely revisit this question. One idea would be for EF Core to provide the comparers for the System.Text.Json types, but without actually using them by default. Users would thus be able to easily opt into automatic change tracking by configuring their property with these built-in comparers, as long as they're aware of the perf impact. Of course, we can also implement these comparers in our providers now without waiting for 6.0. If you want to do this, I'll be happy to take them into Npgsql, or we can do it vice versa too.

On a somewhat-related note, there's the question of sending partial updates when a change is detected. The way things currently work is that once anything in the JSON document changes, the whole value must be replaced - which is bad for big documents. https://github.com/npgsql/efcore.pg/issues/1466 tracks trying to do better - I also hope to look into this for 6.0.

[roji]

/cc @ajcvickers

[lauxjpn]

Cross-database JSON support is likely to be one of my focuses for EF Core 6.0 (dotnet/efcore#4021), so we'll likely revisit this question. One idea would be for EF Core to provide the comparers for the System.Text.Json types, but without actually using them by default.

What is the general approach for usability/performance trade-off scenarios in EF Core? Is it performance first and usability opt-in, or the other way around?

In addition to providing one or multiple value comparers that users then manually could opt into, I would also implement an option for Pomelo that controls if and how they are applied by default. I would also make the "how" part part of the value comparer(s), so users could opt in into specific behaviors like IClonable support etc. (see https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/pull/1159#discussion_r485554333).

For now I recommend to users to flag the change manually (see npgsql/efcore.pg#1228 (comment) for example).

Without having value comparere(s) applied, a simple way to manually check for changes (like an extension method) should be provided, so JSON mapped properties can be checked on demand. Otherwise, if a POCO is being processed somewhere deep inside the app, users might not know if it has been changed or not and would like a simple and tested way to find out on demand, instead of rolling out their own comparison implementation.

Of course, we can also implement these comparers in our providers now without waiting for 6.0. If you want to do this, I'll be happy to take them into Npgsql, or we can do it vice versa too.

I think that is a good idea. With automatic change tracking behavior enabled for normal entities, and JSON POCOs being similar in concept, its probably unexpected to users that there is no simple way that lets changes be automatically discovered for JSON POCOs as well.

Since I am already discussing performance optimizations with @AndriySvyryd in #1159, I would give their implementation a go.

[roji]

What is the general approach for usability/performance trade-off scenarios in EF Core? Is it performance first and usability opt-in, or the other way around?

I think that's a question that doesn't have a single answer - it's rather case-by-case. Of course we always prioritize correctness and then usability, but where something has a big perf cost we may make it opt-in as I'm proposing here. A good example might be the whole case-sensitivity discussion: lots of users have asked "just give me case-insensitive searching by automatically lower-casing my strings" (or similar). While we could do that, it would kill index usage right off the bat, making it a very poor choice perf-wise even if it's more usable etc.

BTW the decision to not provide comparers by default for JSON is only my own, in Npgsql - this hasn't been discussed with the team or anything. But EF itself doesn't do value comparison for byte arrays, for example (see https://github.com/dotnet/efcore/issues/11073), so I think that may in line with that.

Thinking about this again, IIRC JsonDocument/JsonElement are actually immutable, so we should definitely be doing automatic change tracking on them (it's free)...

I would also implement an option for Pomelo that controls if and how they are applied by default

That's definitely possible. Just keep in mind that we may revisit this more globally in 6.0, and so it may be a good idea to adjust the work we've already done at that point...

Without having value comparere(s) applied, a simple way to manually check for changes (like an extension method) should be provided, so JSON mapped properties can be checked on demand. Otherwise, if a POCO is being processed somewhere deep inside the app, users might not know if it has been changed or not and would like a simple and tested way to find out on demand, instead of rolling out their own comparison implementation.

Well, for POCO mapping the problem is essentially snapshotting an arbitrary .NET object graph, and then recursively comparing the first graph to the second - this is obviously not a trivial task, and is bound to have a non-trivial perf impact (even if we go fancy and code-generate the snapshotting and comparison logic).

So while I agree that the lack of automatic change tracking is surprising and not ideal, users seem to have been receptive to manually flagging the entire document as modified...

Let me know what you think we should do here.

[lauxjpn]

I guess it makes sense to have the comparison opt-in by default. We should consider adding an extension method at the property level like UseJsonChangeTracking(someOptions), that just adds the value comparer(s). That would help users discover the opt-in functionality.

That's definitely possible. Just keep in mind that we may revisit this more globally in 6.0, and so it may be a good idea to adjust the work we've already done at that point

I think that is okay with us. Our approach should be to adopt between major releases. We will also prefix our specific implementation with MySql.

Well, for POCO mapping the problem is essentially snapshotting an arbitrary .NET object graph, and then recursively comparing the first graph to the second - this is obviously not a trivial task, and is bound to have a non-trivial perf impact (even if we go fancy and code-generate the snapshotting and comparison logic).

You are right. The snapshot is the issue here that makes this hard to implement.

An approach could be to introduce an IDisposable based scope type that could then be used in a using statement (or manually disposed later). It would take and keep a snapshot at its initialization and do the change tracking comparison and modification state updating later when it is being disposed. Optionally, a couple of methods, to manually control this process could also be added to the type. This would allow for on demand snapshotting and change tracking.

Thinking about this again, IIRC JsonDocument/JsonElement are actually immutable, so we should definitely be doing automatic change tracking on them (it's free)...

Yeah, @AndriySvyryd came to the same conclusion in https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/pull/1159#discussion_r485354010.

[roji]

I guess it makes sense to have the comparison opt-in by default.

You mean not having the value converters by default, requiring a user opt-in for them, right? :)

Another option here is to use change-tracking proxies - make all your POCOs signal to EF Core that the document changed as soon as anything, anywhere is mutated. It could be a good fit for JSON.

Yeah, @AndriySvyryd came to the same conclusion in #1159 (comment).

Oh I didn't notice that, great :) I'll definitely be happy to copy your comparers into EFCore.PG for 5.0 - ping me when that's done.

[mguinness]

Nothing to add here, but wanted to thank everyone working to resolve the complexities involved w/ JSON support.

[lauxjpn]

@roji The JsonDocument and JsonElement related value comparers seem trivial to implement:

```c#
public class JsonDocumentValueComparer: ValueComparer
{
public JsonDocumentValueComparer()
: base(
(left, right) => object.Equals(left, right),
v => v.GetHashCode())
{
}
}

public class JsonElementValueComparer: ValueComparer
{
public JsonElementValueComparer()
: base(
(left, right) => left.Equals(right),
v => v.GetHashCode())
{
}
}

We could even use the default implementation:

```c#
public class JsonDocumentValueComparer: ValueComparer<JsonDocument>
{
    public JsonDocumentValueComparer()
        : base(false)
    {
    }
}

public class JsonElementValueComparer: ValueComparer<JsonElement>
{
    public JsonElementValueComparer()
        : base(false)
    {
    }
}

The base constructor ValueComparer<T>ValueComparer(bool favorStructuralComparisons) would hook them up automatically.

As far as I can see, that is really all there is to the actual value comparer implementations for the Microsoft JSON DOM, as long as we don't want to check them semantically (which we don't by default, because that could be potentially slow again).

[roji]

Yeah, you're right that since JsonDocument/JsonElement don't override Equals, the default implementation should work fine. In fact, unless I'm mistaken, the default implementation is what we get when no comparer is explicitly passed, so without doing anything we already get the above.

However, while this all works great for JsonDocument, JsonElement is a struct - and so things become quite a bit more complicated, because there's no identity/reference equality. Of course, it's still possible to do a recursive structural comparison, but we're basically back to the deep snapshot/comparison perf issues of POCOs. So unless I've missed something, mapping directly to JsonElement should probably be discouraged; in fact, it was probably a mistake to allow mapping it in the first place (I'd consider removing that support from your provider). The main advantage mapping JsonElement provides is not having to call RootElement on JsonDocument, which really is a tiny bit of sugar.

[roji]

(opened https://github.com/npgsql/efcore.pg/issues/1499 to consider deprecating/removing support for mapping JsonElement)

[lauxjpn]

However, while this all works great for JsonDocument, JsonElement is a struct - and so things become quite a bit more complicated, because there's no identity/reference equality.

Not sure yet if this is actually the case. See https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/pull/1159#discussion_r487473465.
There should be indirect referential integrity due to the JsonElement._parent field (which is of type JsonDocument), which should be copied when assigning/snapshotting a JsonElement. This field and the _idx field are the only ones, and together only 4 bytes larger than a simple reference, so this should still be good enough performance-wise.

In case I did overlook something here and it should turn out to be an issue after all, we would remove support for JsonElement altogether for now in Pomelo, and don't even introduce it. As you already pointed out, there is really not much benefit in supporting it directly as the JSON root property anyway.

[roji]

@lauxjpn you're right - always good to look at the source. One bad thing is that JsonElement does not override Equals; this means that the inherited object.Equals is used, which is considered inefficient for structs. So JsonElement is still worse than JsonDocument, but this is constant inefficiency, far from the kind of recursive inefficiency we're trying to avoid. And one may hope that they override it in the future.

[lauxjpn]

One bad thing is that JsonElement does not override Equals; this means that the inherited object.Equals is used, which is considered inefficient for structs.

@roji It might be worth opening an issue about that on the runtime repo. I don't see a reason why JsonElement shouldn't override Equals.

I guess it is out of the question for public packages like ours to implement a custom value comparer, that builds its equals expression in a way that it explicitly compares two JsonElement instances to each other by comparing their two private fields (assuming this would be faster than the default implementation of ValueType.Equals())?

---

The article Performance implications of default struct equality in C# backs-up your statement about the inefficiency of ValueType.Equals().

There seems to be an optimization, that will be used in cases where the pack layout of the struct allows it to, which it does in the JsonElement case (first field 64 or 32 bits, second field 32 bits), the struct doesn't contain any floating point numbers, which JsonElement doesn't, and where the struct contains no references, which JsonElement does however.
This is also backed by the docs about ValueType.Equals():

Tip

Particularly if your value type contains fields that are reference types, you should override the Equals(Object) method. This can improve performance and enable you to more closely represent the meaning of equality for the type.

[roji]

I guess it is out of the question for public packages like ours to implement a custom value comparer, that builds its equals expression in a way that it explicitly compares two JsonElement instances to each other by comparing their two private fields (assuming this would be faster than the default implementation of ValueType.Equals())?

The comparer could definitely access the private fields via reflection, but I wouldn't go there in a comparer. This really is something that should be fixed in JsonElement itself as you propose.

Do you want to submit a PR on this on runtime? If you're busy I can also do something.

[lauxjpn]

Do you want to submit a PR on this on runtime? If you're busy I can also do something.

Go ahead. I currently try to finishing the JSON change tracking implementation, so we can make this part of 3.2.0, which we will release later today.

[roji]

Nothing is going to make it in for 5.0 at this point in any case, so there's no rush. I'd just keep the comparers as the default, which would be a bit inefficient for JsonElement but I'd say it's acceptable.

[lauxjpn]

Go ahead. I currently try to finishing the JSON change tracking implementation, so we can make this part of 3.2.0, which we will release later today.

Nothing is going to make it in for 5.0 at this point in any case, so there's no rush. I'd just keep the comparers as the default, which would be a bit inefficient for JsonElement but I'd say it's acceptable.

I know, it was more directed to JSON change tracking support in general for Pomelo, not to an optimized version of JsonElement.

[roji]

Oh OK, no problem.