Fix compiled-model CreateRelationalModel() failure when an owned entity shares its owner's table#38428
Fix compiled-model CreateRelationalModel() failure when an owned entity shares its owner's table#38428Copilot wants to merge 6 commits into
Conversation
…owner's table Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a runtime failure when creating the relational model from a compiled model in scenarios where an owned entity shares its owner’s table and the owner has an index/constraint. The fix ensures table constraints/indexes/triggers are generated only after all column mappings exist, preventing empty-sequence failures during value-factory setup.
Changes:
- Updated
RelationalCSharpRuntimeAnnotationCodeGeneratorto generate unique constraints, indexes, and triggers in a dedicated post-pass overmodel.Tables, after all column mappings are emitted (and before FK constraints). - Added a regression test (
Owned_entity_sharing_table_with_owner_that_has_an_index) to force relational-model creation in the problematic mapping order and validate index presence. - Regenerated/updated compiled-model scaffolding baselines (primarily reordering constraint/index emission after mappings) and added new Sqlite/SqlServer baselines for the new test.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs | Moves table constraint/index/trigger emission to a table-level post-pass after column mappings are generated. |
| test/EFCore.Relational.Specification.Tests/Scaffolding/CompiledModelRelationalTestBase.cs | Adds regression test and supporting Blog/Post/Author types for the owned-shared-table + index scenario. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Tpc_Sprocs/DbContextModelBuilder.cs | Baseline update reflecting reordered constraint/index emission. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/ComplexTypes/DbContextModelBuilder.cs | Baseline update reflecting reordered constraint/index emission. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/DbContextModelBuilder.cs | Baseline update reflecting reordered constraint/index emission. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/BigModel_with_JSON_columns/DbContextModelBuilder.cs | Baseline update reflecting reordered constraint/index emission. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/DbContextAssemblyAttributes.cs | New baseline assembly attributes for the SqlServer provider. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/DbContextModel.cs | New baseline runtime model for the SqlServer provider. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/DbContextModelBuilder.cs | New baseline relational model builder output for the SqlServer provider. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/BlogEntityType.cs | New baseline entity type for Blog (SqlServer). |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/BlogUnsafeAccessors.cs | New baseline unsafe accessors for Blog (SqlServer). |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/PostEntityType.cs | New baseline entity type for Post (SqlServer). |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/PostUnsafeAccessors.cs | New baseline unsafe accessors for Post (SqlServer). |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/AuthorEntityType.cs | New baseline entity type for owned Author (SqlServer). |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/AuthorUnsafeAccessors.cs | New baseline unsafe accessors for Author (SqlServer). |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/DbContextAssemblyAttributes.cs | New baseline assembly attributes for the Sqlite provider. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/DbContextModel.cs | New baseline runtime model for the Sqlite provider. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/DbContextModelBuilder.cs | New baseline relational model builder output for the Sqlite provider. |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/BlogEntityType.cs | New baseline entity type for Blog (Sqlite). |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/BlogUnsafeAccessors.cs | New baseline unsafe accessors for Blog (Sqlite). |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/PostEntityType.cs | New baseline entity type for Post (Sqlite). |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/PostUnsafeAccessors.cs | New baseline unsafe accessors for Post (Sqlite). |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/AuthorEntityType.cs | New baseline entity type for owned Author (Sqlite). |
| test/EFCore.Sqlite.FunctionalTests/Scaffolding/Baselines/Owned_entity_sharing_table_with_owner_that_has_an_index/AuthorUnsafeAccessors.cs | New baseline unsafe accessors for Author (Sqlite). |
| foreach (var foreignKey in table.ForeignKeyConstraints) | ||
| { | ||
| Create(foreignKey, parameters with { TargetName = parameters.ScopeVariables[table] }); | ||
| } |
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
| foreach (var table in model.Tables) | ||
| { | ||
| foreach (var foreignKey in table.ForeignKeyConstraints) | ||
| { | ||
| Create(foreignKey, parameters with { TargetName = parameters.ScopeVariables[table] }); | ||
| } | ||
| CreateTableConstraints(table, relationalModelParameters); | ||
| } | ||
|
|
||
| foreach (var table in model.Tables) | ||
| { | ||
| CreateTableConstraints(table, relationalModelParameters, foreignKeysOnly: true); | ||
| } |
There was a problem hiding this comment.
@copilot Collapse these loops and remove foreignKeysOnly parameter. Update baselines
There was a problem hiding this comment.
Removed the foreignKeysOnly parameter — the FK pass is now inlined directly in CreateRelationalModel (matching the pre-existing style for the other table loops).
I kept the foreign keys in a separate model.Tables loop rather than collapsing the two into one, because a single interleaved pass breaks at runtime. Each generated foreign key resolves its principal's unique constraint eagerly via principalTable.FindUniqueConstraint(name)!, so every unique constraint must already be emitted before any foreign key runs. IRelationalModel.Tables is insertion/alphabetically ordered (Dictionary<(string, string?), Table>), not topologically ordered, so when a dependent table is processed before its principal, FindUniqueConstraint returns null and CreateRelationalModel() throws NullReferenceException.
I reproduced this concretely: collapsing into one loop makes CompiledModelSqliteTest.BigModel_with_JSON_columns throw NullReferenceException from GetRelationalModel() (e.g. dependentBase's FK referencing principalBase.FindUniqueConstraint("AK_PrincipalBase_Id") before principalBase's constraints are added). Keeping the FK pass after the unique-constraint/index/trigger pass keeps it green.
Because the relative emission order is unchanged, no baselines needed updating. Let me know if you'd prefer a different structure (e.g. a dedicated CreateForeignKeyConstraints method) or if you'd like me to pursue topologically ordering the tables instead.
There was a problem hiding this comment.
@copilot Ok, then revert the fk generation code and separate it. Add a comment explaining why it's a different loop
There was a problem hiding this comment.
Done in c814f17. Reverted the FK generation back to the original Create(foreignKey, parameters with { TargetName = parameters.ScopeVariables[table] }) call, kept it as a separate model.Tables loop, and added a comment explaining that it runs after the unique-constraint pass because each FK resolves the principal table's unique constraint, which may belong to a table processed later above. Generated output is unchanged, so no baselines needed updating.
There was a problem hiding this comment.
Correction: the commit is 6895dee (not c814f17).
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
…atory comment Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
A compiled model whose owned entity shares its owner's table (where the owner has an index/constraint) threw
System.InvalidOperationException: Sequence contains no elementsfromCreateRelationalModel()at runtime, despite compiling successfully.Root cause
RelationalCSharpRuntimeAnnotationCodeGeneratoremitted unique-constraint/index/trigger creation inline, guarded bytableMapping == table.EntityTypeMappings.Last(). When an owned entity (e.g.Author) shares its owner's (Post) table and its mapping is the lastEntityTypeMappingbut is iterated before the owner, an index (e.g.IX_Posts_BlogId) was generated before the referenced column's mapping existed.SetRowIndexValueFactory(...)then hitColumnBase.GetDefaultStoreTypeMapping(), which requires at least one column mapping — hence the empty-sequence failure.Changes
RelationalCSharpRuntimeAnnotationCodeGenerator: moved unique-constraint, index, and trigger emission out of the per-ITableMappingpath into aCreateTableConstraintspass overmodel.Tables, run after all column mappings are emitted, guaranteeing every referenced column is mapped before constraints/indexes are built.model.Tablespass afterCreateTableConstraints. Generated FKs resolve their principal's unique constraint at runtime viaprincipalTable.FindUniqueConstraint(name)!, andIRelationalModel.Tablesis not topologically ordered — so all unique constraints must be emitted before any FK. TheforeignKeysOnlyparameter was removed; the FK loop is inlined.Owned_entity_sharing_table_with_owner_that_has_an_indexregression test (Blog/Post/Author).BigModel_with_JSON_columns,ComplexTypes,Tpc_Sprocs) — pure relocations of constraint/index emission after column mappings.Generated code, before vs after