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

NRE when adding entity with an owned entity and ToJson configuration #32288

Closed
markushaslinger opened this issue Nov 13, 2023 · 7 comments
Closed
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@markushaslinger
Copy link

@roji asked me to open this issue, might be a simple configuration error, though.

Scenario

A very bare bones console application I created to check out the new ToJson support in npgsql.
Uses the following simple model to create and insert two students, then performs a query on the date field of the owned entity.
Mind that the owned entity contains a NodaTime type property.

public class Student
{
    public int Id { get; set; }
    public required string Name { get; set; }
    public required List<ExamGrade> ExamGrades { get; set; }
}

public sealed class ExamGrade
{
    public Grade Grade { get; set; }
    public required string Subject { get; set; }
    public LocalDate Date { get; set; } // NodaTime type
}

public enum Grade
{
    A,
    // ...
}

Behavior

A NullReferenceException is thrown when calling SaveChanges.

Expected Behavior

No exception is thrown, entities are inserted and then queried successfully.

Exception

An exception occurred in the database while saving changes for context type 'JsonColumnTest.TestContext'.
      System.NullReferenceException: Object reference not set to an instance of an object.
         at Microsoft.EntityFrameworkCore.Update.ModificationCommand.WriteJson(Utf8JsonWriter writer, Object navigationValue, IUpdateEntry parentEntry, IEntityType entityType, Nullable`1 ordinal, Boolean isCollection, Boolean isTopLevel)
         at Microsoft.EntityFrameworkCore.Update.ModificationCommand.WriteJson(Utf8JsonWriter writer, Object navigationValue, IUpdateEntry parentEntry, IEntityType entityType, Nullable`1 ordinal, Boolean isCollection, Boolean isTopLevel)
         at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<GenerateColumnModifications>g__HandleJson|41_3(List`1 columnModifications, <>c__DisplayClass41_0&)
         at Microsoft.EntityFrameworkCore.Update.ModificationCommand.GenerateColumnModifications()
         at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<>c.<get_ColumnModifications>b__33_0(ModificationCommand command)
         at Microsoft.EntityFrameworkCore.Internal.NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory)
         at Microsoft.EntityFrameworkCore.Update.ModificationCommand.get_ColumnModifications()
         at Npgsql.EntityFrameworkCore.PostgreSQL.Update.Internal.NpgsqlUpdateSqlGenerator.AppendInsertOperation(StringBuilder commandStringBuilder, IReadOnlyModificationCommand command, Int32 commandPosition, Boolean overridingSystemValue, Boolean& requiresTransaction)
         at Npgsql.EntityFrameworkCore.PostgreSQL.Update.Internal.NpgsqlUpdateSqlGenerator.AppendInsertOperation(StringBuilder commandStringBuilder, IReadOnlyModificationCommand command, Int32 commandPosition, Boolean& requiresTransaction)
         at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.AddCommand(IReadOnlyModificationCommand modificationCommand)
         at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.TryAddCommand(IReadOnlyModificationCommand modificationCommand)
         at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.CreateCommandBatches(IEnumerable`1 commandSet, Boolean moreCommandSets, Boolean assertColumnModification, ParameterNameGenerator parameterNameGenerator)+MoveNext()
         at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IList`1 entries, IUpdateAdapter updateAdapter)+MoveNext()
         at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
         at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IList`1 entries)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(StateManager stateManager, Boolean acceptAllChangesOnSuccess)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<>c.<SaveChanges>b__112_0(DbContext _, ValueTuple`2 t)
         at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
         at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
         at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.WriteJson(Utf8JsonWriter writer, Object navigationValue, IUpdateEntry parentEntry, IEntityType entityType, Nullable`1 ordinal, Boolean isCollection, Boolean isTopLevel)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.WriteJson(Utf8JsonWriter writer, Object navigationValue, IUpdateEntry parentEntry, IEntityType entityType, Nullable`1 ordinal, Boolean isCollection, Boolean isTopLevel)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<GenerateColumnModifications>g__HandleJson|41_3(List`1 columnModifications, <>c__DisplayClass41_0&)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.GenerateColumnModifications()
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.<>c.<get_ColumnModifications>b__33_0(ModificationCommand command)
   at Microsoft.EntityFrameworkCore.Internal.NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory)
   at Microsoft.EntityFrameworkCore.Update.ModificationCommand.get_ColumnModifications()
   at Npgsql.EntityFrameworkCore.PostgreSQL.Update.Internal.NpgsqlUpdateSqlGenerator.AppendInsertOperation(StringBuilder commandStringBuilder, IReadOnlyModificationCommand command, Int32 commandPosition, Boolean overridingSystemValue, Boolean& requiresTransaction)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Update.Internal.NpgsqlUpdateSqlGenerator.AppendInsertOperation(StringBuilder commandStringBuilder, IReadOnlyModificationCommand command, Int32 commandPosition, Boolean& requiresTransaction)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.AddCommand(IReadOnlyModificationCommand modificationCommand)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.TryAddCommand(IReadOnlyModificationCommand modificationCommand)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.CreateCommandBatches(IEnumerable`1 commandSet, Boolean moreCommandSets, Boolean assertColumnModification, ParameterNameGenerator parameterNameGenerator)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IList`1 entries, IUpdateAdapter updateAdapter)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(StateManager stateManager, Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<>c.<SaveChanges>b__112_0(DbContext _, ValueTuple`2 t)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()
   at Program.<Main>$(String[] args) in F:\temp\tojsontest\Program.cs:line 39

Reproduction

I pushed the project to this repo. It should be sufficient to clone and run to get the exception.
It is set up to use Postgres, but a quick test showed the same error when using SqlServer - which is why I'm opening the issue here and not in the efcore.pg repo.

Workaround

Defining a Converter using JsonSerializer allows the entities to be inserted and the query to be excuted, but the date column is of course treated as string and not as date.
Sample in the repro project as well.

Provider and version information

EF Core version: 8.0.0-rc.2.23480.1
Database provider: PostgreSQL 8.0.0-rtm-ci.20231113T150204, SqlServer 8.0.0-rc.2.23480.1
Target framework: net8
Operating system: Win11 x64
IDE: Rider
NodaTime Compat: Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime 8.0.0-rtm-ci.20231113T150204, SimplerSoftware.EntityFrameworkCore.SqlServer.NodaTime 8.0.0-rc.2.23480.1

@ajcvickers
Copy link
Member

@roji Is this just about a better exception when a mapped type doesn't have a JSON serializer?

@roji
Copy link
Member

roji commented Nov 15, 2023

Ah yes, good catch. I added the JSON support to most type mappings in EFCore.PG after rc2 (npgsql/efcore.pg#2926). I've already added fixes to throw a better exception in #32255 (which needs to be completed).

@markushaslinger can you please try Npgsql.EntityFrameworkCore.PostgreSQL 8.0.0-rtm-ci.20231115T131924 from the daily build vNext feed, and report on how that works for you? You should be able to use that with EF 8.0.0 GA.

@markushaslinger
Copy link
Author

@roji I tried, but the closest I could find for Npgsql was 8.0.0-rtm-ci.20231115T161454, so I used that.
Running with these packages gives me the following exception while creating the migration (usuaully I'd assume a version mismatch):

Unhandled exception. System.MissingMethodException: Method not found: 'Void Npgsql.TypeMapping.INpgsqlTypeMapper.AddTypeInfoResolver(Npgsql.Internal.IPgTypeInfoResolver)'.
   at Npgsql.NpgsqlNodaTimeExtensions.UseNodaTime(INpgsqlTypeMapper mapper)
   at Microsoft.EntityFrameworkCore.NpgsqlNodaTimeDbContextOptionsBuilderExtensions.UseNodaTime(NpgsqlDbContextOptionsBuilder optionsBuilder)
   at JsonColumnTest.TestContext.<>c.<OnConfiguring>b__6_0(NpgsqlDbContextOptionsBuilder o) in F:\temp\clonetest\TestContext.cs:line 42
   at Microsoft.EntityFrameworkCore.NpgsqlDbContextOptionsBuilderExtensions.UseNpgsql(DbContextOptionsBuilder optionsBuilder, String connectionString, Action`1 npgsqlOptionsAction)
   at JsonColumnTest.TestContext.OnConfiguring(DbContextOptionsBuilder optionsBuilder) in F:\temp\clonetest\TestContext.cs:line 40
   at Microsoft.EntityFrameworkCore.DbContext.get_ContextServices()
   at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider()
   at Microsoft.EntityFrameworkCore.DbContext.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
   at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
   at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.GetRelationalService[TService](IInfrastructure`1 databaseFacade)
   at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.Migrate(DatabaseFacade databaseFacade)
   at Program.<Main>$(String[] args) in F:\temp\tojsontest\Program.cs:line 6

Used packages are (updated in the sample project I shared as well):

    <PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="8.0.0">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
    <PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="8.0.0" />
    <PackageReference Include="NodaTime" Version="3.1.9" />
    <PackageReference Include="NodaTime.Serialization.SystemTextJson" Version="1.1.2" />
    <PackageReference Include="Npgsql" Version="8.0.0-rtm-ci.20231115T161454" />
    <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="8.0.0-rtm-ci.20231115T131924" />
    <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime" Version="8.0.0-rtm-ci.20231115T131924" />

@roji
Copy link
Member

roji commented Nov 15, 2023

@markushaslinger can you please take another look? I'm seeing the right version (20231115T131924) show up as the latest on the myget vnext feed. I've checked and that version already references EF 8.0.0.

The exact version is important as things are moving pretty fast right now.

@roji
Copy link
Member

roji commented Nov 15, 2023

Also, I recommend removing Npgsql from your csproj, allowing Npgsql.EntityFrameworkCore.PostgreSQL to bring in whatever version it was built against. That may be the reason for the MissingMethodException.

@markushaslinger
Copy link
Author

I'm seeing the right version (20231115T131924) show up as the latest

Yeah, I had the right version of Npgsql.EntityFrameworkCore.PostgreSQL, but the wrong of Npgsql.

Also, I recommend removing Npgsql from your csproj, allowing Npgsql.EntityFrameworkCore.PostgreSQL to bring in whatever version it was built against. That may be the reason for the MissingMethodException.

And that was exactly it, don't know why I started including all three references in the first place, but I then just continued doing so in all subsequent projects for no apparent reason.

Anyway: looks good!
No exceptions and types are picked up with a delicious query as a result:

SELECT s."Id", s."Name", s."ExamGrades"
FROM jsontest."Students" AS s
WHERE EXISTS (
  SELECT 1
  FROM ROWS FROM (jsonb_to_recordset(s."ExamGrades") AS (
	  "Date" date,
	  "Grade" integer,
	  "Subject" text
  )) WITH ORDINALITY AS e
  WHERE e."Date" > @__threshold_0)
LIMIT 1

I think this can be closed, my scenario is working fine at least. Thanks for patching it up so quickly!

@roji
Copy link
Member

roji commented Nov 15, 2023

Great! Thanks for testing and confirming!

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2023
@roji roji unassigned maumar Nov 15, 2023
@roji roji added the closed-no-further-action The issue is closed and no further action is planned. label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

4 participants