Skip to content

Fix NRE in AddJsonNavigationBindings when combining DbFunction with OwnsOne/OwnsMany ToJson #37855

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.

Sign up for GitHub

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

Merged
roji merged 5 commits into from
Mar 6, 2026
+118 −34
Merged

Fix NRE in AddJsonNavigationBindings when combining DbFunction with OwnsOne/OwnsMany ToJson #37855

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1ddc7e7
Initial plan
Copilot Mar 5, 2026
c55a409
Fix NullReferenceException in AddJsonNavigationBindings when combinin…
Copilot Mar 5, 2026
1c902a7
Address review feedback: simplify CreateFunctionMapping and fix GetFu…
Copilot Mar 5, 2026
4bfd2db
Use existing Order class in function+JSON test and assert column names
Copilot Mar 5, 2026
e3ee4f0
Extract AddTvfMapping helper, add OwnsMany test, remove blank line
Copilot Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,20 @@ public static void SetSqlQuery(this IMutableEntityType entityType, string? query
public static string? GetFunctionName(this IReadOnlyEntityType entityType)
{
var nameAnnotation = entityType.FindAnnotation(RelationalAnnotationNames.FunctionName);
return nameAnnotation != null
? (string?)nameAnnotation.Value
: entityType.BaseType != null
? entityType.GetRootType().GetFunctionName()
if (nameAnnotation != null)
{
return (string?)nameAnnotation.Value;
}

if (entityType.BaseType != null)
{
return entityType.GetRootType().GetFunctionName();
}

var ownership = entityType.FindOwnership();
return ownership != null
&& (ownership.IsUnique || entityType.IsMappedToJson())
? ownership.PrincipalEntityType.GetFunctionName()
: null;
}

Expand Down
82 changes: 58 additions & 24 deletions src/EFCore.Relational/Metadata/Internal/RelationalModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ public static IRelationalModel Create(

AddSqlQueries(databaseModel, entityType);

AddMappedFunctions(databaseModel, entityType);
AddMappedFunctions(databaseModel, entityType, relationalTypeMappingSource);

AddStoredProcedures(databaseModel, entityType, relationalTypeMappingSource);
}

AddTvfs(databaseModel);
AddTvfs(databaseModel, relationalTypeMappingSource);

var tables = ((IRelationalModel)databaseModel).Tables;
foreach (Table table in tables)
Expand Down Expand Up @@ -918,7 +918,7 @@ private static void AddSqlQueries(RelationalModel databaseModel, IEntityType ent
queryMappings?.Reverse();
}

private static void AddMappedFunctions(RelationalModel databaseModel, IEntityType entityType)
private static void AddMappedFunctions(RelationalModel databaseModel, IEntityType entityType, IRelationalTypeMappingSource relationalTypeMappingSource)
{
var model = databaseModel.Model;
var functionName = entityType.GetFunctionName();
Expand All @@ -940,7 +940,7 @@ private static void AddMappedFunctions(RelationalModel databaseModel, IEntityTyp
}

var dbFunction = (IRuntimeDbFunction)model.FindDbFunction(mappedFunctionName)!;
var functionMapping = CreateFunctionMapping(entityType, mappedType, dbFunction, databaseModel, @default: true);
var functionMapping = CreateFunctionMapping(entityType, mappedType, dbFunction, databaseModel, relationalTypeMappingSource, @default: true);

mappedType = mappedType.BaseType;

Expand All @@ -963,7 +963,7 @@ private static void AddMappedFunctions(RelationalModel databaseModel, IEntityTyp
functionMappings?.Reverse();
}

private static void AddTvfs(RelationalModel relationalModel)
private static void AddTvfs(RelationalModel relationalModel, IRelationalTypeMappingSource relationalTypeMappingSource)
{
var model = relationalModel.Model;
foreach (IRuntimeDbFunction function in model.GetDbFunctions())
Expand All @@ -982,7 +982,7 @@ private static void AddTvfs(RelationalModel relationalModel)
continue;
}

var functionMapping = CreateFunctionMapping(entityType, entityType, function, relationalModel, @default: false);
var functionMapping = CreateFunctionMapping(entityType, entityType, function, relationalModel, relationalTypeMappingSource, @default: false);

if (entityType.FindRuntimeAnnotationValue(RelationalAnnotationNames.FunctionMappings)
is not List<FunctionMapping> functionMappings)
Expand All @@ -993,6 +993,27 @@ private static void AddTvfs(RelationalModel relationalModel)

functionMappings.Add(functionMapping);
((StoreFunction)functionMapping.StoreFunction).EntityTypeMappings.Add(functionMapping);

foreach (var ownedJsonNavigation in entityType.GetNavigationsInHierarchy()
.Where(
n => n.ForeignKey.IsOwnership
&& n.TargetEntityType.IsMappedToJson()
&& n.ForeignKey.PrincipalToDependent == n))
{
var ownedType = ownedJsonNavigation.TargetEntityType;
var ownedFunctionMapping = CreateFunctionMapping(
ownedType, ownedType, function, relationalModel, relationalTypeMappingSource, @default: false);

if (ownedType.FindRuntimeAnnotationValue(RelationalAnnotationNames.FunctionMappings)
is not List<FunctionMapping> ownedFunctionMappings)
{
ownedFunctionMappings = [];
ownedType.AddRuntimeAnnotation(RelationalAnnotationNames.FunctionMappings, ownedFunctionMappings);
}

ownedFunctionMappings.Add(ownedFunctionMapping);
((StoreFunction)ownedFunctionMapping.StoreFunction).EntityTypeMappings.Add(ownedFunctionMapping);

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The annotation-list initialization + mapping registration logic is duplicated (similar logic exists for the root entityType earlier in AddTvfs). Consider extracting this into a small helper to keep the mapping registration consistent and reduce copy/paste (e.g., a local function that “gets/creates function mappings list, adds mapping, and registers it on the StoreFunction”).

Copilot uses AI. Check for mistakes.

@AndriySvyryd AndriySvyryd Mar 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot The annotation-list initialization + mapping registration logic is duplicated (similar logic exists for the root entityType earlier in AddTvfs). Consider extracting this into a small helper to keep the mapping registration consistent and reduce duplication

Copilot AI Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted AddTvfMapping helper in e3ee4f0. The duplicated annotation-list initialization + mapping registration logic in AddTvfs is now a single method called for both the root entity and each owned JSON type.

Copilot AI Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted AddTvfMapping helper in e3ee4f0.

}
}
}

Expand All @@ -1001,6 +1022,7 @@ private static FunctionMapping CreateFunctionMapping(
IEntityType mappedType,
IRuntimeDbFunction dbFunction,
RelationalModel model,
IRelationalTypeMappingSource relationalTypeMappingSource,
bool @default)
{
var storeFunction = GetOrCreateStoreFunction(dbFunction, model);
Expand All @@ -1010,29 +1032,41 @@ private static FunctionMapping CreateFunctionMapping(
entityType, storeFunction, dbFunction,
includesDerivedTypes: entityType.GetDirectlyDerivedTypes().Any() ? true : null) { IsDefaultFunctionMapping = @default };

foreach (var property in mappedType.GetProperties())
var containerColumnName = mappedType.GetContainerColumnName();
var containerColumnType = mappedType.GetContainerColumnType();
if (!string.IsNullOrEmpty(containerColumnName))
{
var columnName = property.GetColumnName(mappedFunction);
if (columnName == null)
CreateContainerColumn(
storeFunction, containerColumnName, containerColumnType, mappedType, relationalTypeMappingSource,
static (colName, colType, table, mapping)
=> new FunctionColumn(colName, colType ?? mapping.StoreType, (StoreFunction)table, mapping));
}
else
{
foreach (var property in mappedType.GetProperties())
{
continue;
}
var columnName = property.GetColumnName(mappedFunction);
if (columnName == null)
{
continue;
}

var column = storeFunction.FindColumn(columnName);
if (column == null)
{
column = new FunctionColumn(columnName, property.GetColumnType(mappedFunction), storeFunction)
var column = storeFunction.FindColumn(columnName);
if (column == null)
{
IsNullable = property.IsColumnNullable(mappedFunction)
};
storeFunction.Columns.Add(columnName, column);
}
else if (!property.IsColumnNullable(mappedFunction))
{
column.IsNullable = false;
}
column = new FunctionColumn(columnName, property.GetColumnType(mappedFunction), storeFunction)
{
IsNullable = property.IsColumnNullable(mappedFunction)
};
storeFunction.Columns.Add(columnName, column);
}
else if (!property.IsColumnNullable(mappedFunction))
{
column.IsNullable = false;
}

CreateFunctionColumnMapping(column, property, functionMapping);
CreateFunctionColumnMapping(column, property, functionMapping);
}
}

return functionMapping;
Expand Down
51 changes: 51 additions & 0 deletions test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2997,6 +2997,9 @@ private static IQueryable<Order> GetOrdersForCustomer(int id)
private static IQueryable<Order> GetOrdersForCustomer(string name)
=> throw new NotImplementedException();

private static IQueryable<EntityWithJsonOwnedType> GetEntitiesWithJsonOwnedType()
=> throw new NotImplementedException();

[ConditionalFact]
public void Complex_property_container_column_type_is_used_in_relational_model()
{
Expand Down Expand Up @@ -3264,6 +3267,36 @@ public void Complex_property_json_column_is_nullable_in_TPH_hierarchy()
Assert.IsType<JsonColumn>(jsonColumn);
}

[ConditionalFact]
public void Can_use_relational_model_with_functions_and_json_owned_types()
Comment thread
AndriySvyryd marked this conversation as resolved.
{
var modelBuilder = CreateConventionModelBuilder();

modelBuilder.Entity<EntityWithJsonOwnedType>(cb =>

@AndriySvyryd AndriySvyryd Mar 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Try to use existing classes instead of adding EntityWithJsonOwnedType.
Also assert the non-json columns to ensure no extra ones are addded

Copilot AI Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 4bfd2db. Replaced EntityWithJsonOwnedType with existing Order class using OwnsOne(c => c.DateDetails, o => o.ToJson("date_details")), removed the three custom classes and GetEntitiesWithJsonOwnedType method, and added column name assertions to verify no extra columns are present.

{
cb.HasKey(x => x.Id);
#pragma warning disable EF8001 // Owned JSON entities are obsolete
cb.OwnsOne(x => x.OwnedData, o => o.ToJson("owned_data"));
cb.OwnsMany(x => x.OwnedItems, o => o.ToJson("owned_items"));
#pragma warning restore EF8001
});

modelBuilder.HasDbFunction(
typeof(RelationalModelTest).GetMethod(
nameof(GetEntitiesWithJsonOwnedType), BindingFlags.NonPublic | BindingFlags.Static));

var model = Finalize(modelBuilder);

var entityType = model.Model.FindEntityType(typeof(EntityWithJsonOwnedType));

var functionMappings = entityType.GetFunctionMappings().ToList();
Assert.Single(functionMappings);

var storeFunction = functionMappings[0].StoreFunction;
Assert.NotNull(storeFunction.FindColumn("owned_data"));
Assert.NotNull(storeFunction.FindColumn("owned_items"));
}

private static IRelationalModel Finalize(TestHelpers.TestModelBuilder modelBuilder)
=> modelBuilder.FinalizeModel(designTime: true).GetRelationalModel();

Expand Down Expand Up @@ -3407,6 +3440,24 @@ private class ComplexData
public string Value { get; set; }
public int Number { get; set; }
}

@AndriySvyryd AndriySvyryd Mar 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Remove blank line

Copilot AI Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in e3ee4f0.

private class EntityWithJsonOwnedType
{
public int Id { get; set; }
public string Name { get; set; }
public OwnedJsonData OwnedData { get; set; }
public ICollection<OwnedJsonItem> OwnedItems { get; set; }
}

private class OwnedJsonData
{
public string Value { get; set; }
}

private class OwnedJsonItem
{
public string Description { get; set; }
}
}
}

Expand Down
Loading