Skip to content

Conversation

@Lion-Ch
Copy link

@Lion-Ch Lion-Ch commented Nov 25, 2025

Fixed the case of inserting child entities, when there could be an entity with an empty child entity at the beginning of the array. Because of this, the subsequent insertion of bundles of entities was performed incorrectly. GroupBy by EntityType in ExecuteWithGraphAsync put the type of entity that was the parent in the first place.

Example

The main entity is Parent - P, the child entity is Child - C.
If there was an array Parent[]

[
new () { Child = null },
new () { Child = new() },
new () { Child = new() },
// ...
]

After sorting, we get: PCPCP

After grouping, we will insert in bundles: Parent[], Child[]. Which is wrong!

If all the Parents had Child, then after sorting we get: CPCPCP
After grouping, we will insert in bundles: Child[], Parent[]. That's right.

Solution

To avoid this problem, an additional sorting by graph depth was added.
For each entity, its depth (length to the vertex) is determined. The maximum depth is determined for each type of entity in order to understand which entities need to be inserted first (the type of entity with the maximum depth) and which last (the type of entity with the minimum depth).

  1. For example, after sorting, we get: PCPCP
  2. After determining the depth: P(0)C(1)P(0)C(1)P(0)
  3. After determining the maximum depth for the type of entity: P = 0, C = 1
  4. After applying depth sorting, we get: C(1)C(1)P(0)P(0)P(0)

Further, with GroupBy, the groups will be formed correctly and the identifiers for external entities will be entered correctly.

In summary

  • Added additional sorting of the graph by depth
  • Added a test to check the insertion of entities (BulkInsertOrUpdate_EntityWithNestedNullableObjectGraph_SavesGraphToDatabase1)
  • To activate the old logic, disable withAdditionalSorting in GraphUtil

It remains to be done

  • Roll back the database schema and write a new test
  • Consider the cases I haven’t accounted for, because this error is the only one I have fixed.
  • Add tests for more complex entity graphs and test the algorithm.
  • Test performance on large amounts of inserted data (reflection, algorithmic complexity)

@borisdj borisdj merged commit 5bd2fd0 into borisdj:EFCore8.0 Jan 7, 2026
@borisdj
Copy link
Owner

borisdj commented Jan 7, 2026

Merged, thx for Contrib.

@Lion-Ch
Copy link
Author

Lion-Ch commented Jan 7, 2026

@borisdj This merge request is incomplete (not all the tasks I listed at the end of the issue have been completed). I would recommend rolling it back and marking it as a draft.

@borisdj
Copy link
Owner

borisdj commented Jan 7, 2026

@Lion-Ch
I thing we can keep it, but will try to make upgrades soon, and fast fixes if any issues arise.
Also maybe you could make another PR to add make this SortingFeature configurabe in BulkConfig with default to False (for now, later maybe default could be True), but still to be able to switch of if not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants