From c26c169dbec7a99b0f85b2509d0d7331ea25d996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Tue, 22 Sep 2026 21:22:18 +0200 Subject: [PATCH 1/3] fix: check complex index names in the provider's namespace; open 5.4.1 Both core name checks grouped by table, schema and name, which is how SQL Server scopes index names but not SQLite: SQLite keeps them unique across the database and leaves configured schemas out of the DDL. Two tables sharing an index name - two complex indexes, or a complex and a native one - scaffolded cleanly and failed when applied ("index ... already exists"), and IndexNameCollisionTests asserted that exact model was fine. IndexNameScope (Table, Schema, Database) is now a protected virtual on the core differ, applied by ValidateUniqueIndexNames and ValidateNoNativeIndexNameCollision. The core default is Database, SQLite's rule and the widest scope, since the core also serves providers without a satellite; SQL Server overrides to Table, PostgreSQL to Schema. On PostgreSQL the core now reports plain index clashes first; ValidateNamesAcrossKinds still covers keys, constraint-backed indexes and an unset schema against an explicit 'public'. Only collisions with one of this package's declarations are reported, in the target model only. Reverting the source fails 5 tests: the 3 SQLite rejections (no exception thrown) and 2 PostgreSQL message assertions, which 5.4.0 answered from the cross-kind check. Dropping the SQL Server or PostgreSQL override, or defaulting the core to Schema, each fails the tests pinning it. Version 5.4.1, package-validation baseline 5.4.0. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 + CLAUDE.md | 23 +- Directory.Build.props | 4 +- README.md | 10 +- ...NpgsqlComplexIndexMigrationsModelDiffer.cs | 15 ++ .../CHANGELOG.md | 6 + ...ServerComplexIndexMigrationsModelDiffer.cs | 6 + src/EFCore.ComplexIndexes/CHANGELOG.md | 12 + .../CustomMigrationsModelDiffer.cs | 79 ++++-- src/EFCore.ComplexIndexes/IndexNameScope.cs | 21 ++ src/EFCore.ComplexIndexes/README.md | 5 +- .../IndexNameCollisionTests.cs | 233 +++++++++++++++++- .../NpgsqlNameCollisionTests.cs | 29 ++- 13 files changed, 408 insertions(+), 42 deletions(-) create mode 100644 src/EFCore.ComplexIndexes/IndexNameScope.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bd52aa..3959551 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ covering only what changed for that package: [PostgreSQL](src/EFCore.ComplexIndexes.PostgreSQL/CHANGELOG.md), [SQL Server](src/EFCore.ComplexIndexes.SqlServer/CHANGELOG.md). +## 5.4.1 + +One fix, to the index-name check: it compared names per table on every provider, so on SQLite a name +reused on another table scaffolded cleanly and failed when the migration was applied. + +- **Fixed:** on SQLite, a complex index name is checked across the whole database at `migrations add`, not per table. SQLite rejects a second `CREATE INDEX` under a name already used on any other table, and ignores the schemas a model configures, so two tables sharing an index name — two complex indexes, or a complex index and a native `HasIndex` — scaffolded cleanly and failed when applied ("index … already exists"). The scope is now the provider's: per database in the core, which also serves providers without a satellite; per schema on PostgreSQL; per table on SQL Server, where reusing a name across tables stays allowed. **Upgrading:** a SQLite model that reuses a complex index name across tables now fails at `migrations add`, naming both tables; rename one. On a provider without a satellite that scopes names per table, such as MySQL, that rename is one the database would not have needed, and with the differ registered at runtime (`UseComplexIndexes()`) `Migrate()` raises the same error until it is made. A snapshot holding such a name stays diffable. + ## 5.4.0 Fixes to what 5.x already ships, found while testing the packages against EF Core 11 and planning diff --git a/CLAUDE.md b/CLAUDE.md index 8452553..8740893 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -267,7 +267,7 @@ stored as property annotations) and entity-level `HasComplexIndex(x => x.Complex Names are validated at **two** levels, because neither alone is sufficient. `AddOrReplace` rejects an explicit name already used on the same entity (fast feedback at the declaration), and the -differ's `ValidateUniqueIndexNames` rejects duplicate resolved names per table — the only place +differ's `ValidateUniqueIndexNames` rejects duplicate resolved names within the provider's scope (below) — the only place that sees across the two stores (property-level annotations vs the entity-level list) and knows the *default* names, which depend on resolved column names. It validates the **target model only**: a snapshot that already contains a collision must stay diffable, or the model could never be fixed. @@ -293,8 +293,23 @@ applied clean), and index names per **schema**, where every primary key, unique, temporal constraint also owns an index (42P07). A collision is reported only when one party is this package's; two of EF's own objects are EF's business. It collects into a list, not a set: the two same-named declarations it exists to find produce identical entries, and a set merged them. -Core's `ValidateUniqueIndexNames` stays per table on purpose — SQL Server scopes index names per -table. + +Where an index name must be unique is a provider fact, declared through +`CustomMigrationsModelDiffer.IndexNameScope` and applied by both core checks +(`ValidateUniqueIndexNames`, `ValidateNoNativeIndexNameCollision`): `IndexNameScope.Database` by +default, `IndexNameScope.Schema` in the PostgreSQL satellite, `IndexNameScope.Table` in the SQL +Server one. The default is SQLite's rule, and it has to be *database*, not schema: SQLite's provider +keeps a configured schema in the model but leaves it out of the DDL, so `north.a` and `south.b` +share one namespace (probed 2026-09-22). Until 5.4.1 both checks were per table everywhere, and the +SQLite test "the same name on a different table is not a collision" approved a model that +`sqlite3` rejects with "index … already exists". The widest scope is also the right default for +providers without a satellite: too wide costs a rename the database did not need (MySQL scopes per +table), too narrow a failed apply. On PostgreSQL a clash between two plain indexes is therefore +reported by the core, before `ValidateNamesAcrossKinds` runs; the cross-kind check keeps complex +indexes in its list for the pairs only it sees — keys, constraint-backed indexes, and a table in the +unset default schema against one naming `public`, which the core compares as configured. Each +collision throws once, from whichever check sees it first; do not "deduplicate" by filtering index +pairs out of the cross-kind check, since it is the only net for the `public` case. ### The read model is the differ's reader @@ -493,7 +508,7 @@ still skipped silently — an index on those is nothing this package could creat ### Key extension points -- **Adding a new provider**: Subclass `CustomMigrationsModelDiffer` (override `IsForwardedIndexAnnotation`, optionally `ValidateCreateIndexOperation`/`ResolveUnmappedPart`/`QuoteIdentifier`), implement `IDesignTimeServices` to replace the differ, and ship a `.targets` file that injects the attribute (with `ForProvider` set). The PostgreSQL project is the full-featured reference; the SQL Server project is the minimal one (whitelist + validation, no custom SQL generator). +- **Adding a new provider**: Subclass `CustomMigrationsModelDiffer` (override `IsForwardedIndexAnnotation` and `IndexNameScope` unless the provider keeps index names unique across the database, optionally `ValidateCreateIndexOperation`/`ResolveUnmappedPart`/`QuoteIdentifier`), implement `IDesignTimeServices` to replace the differ, and ship a `.targets` file that injects the attribute (with `ForProvider` set). The PostgreSQL project is the full-featured reference; the SQL Server project is the minimal one (whitelist + validation, no custom SQL generator). - **New index options**: Add constants to `ComplexIndexAnnotations.cs` (or `NpgsqlAnnotations.cs`), expose them via `ComplexIndexBuilder`, and read them in the differ when constructing `CreateIndexOperation`. ### Expression path extraction diff --git a/Directory.Build.props b/Directory.Build.props index 96ef5bb..41f9400 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,6 +1,6 @@ - 5.4.0 + 5.4.1 CaffeinatedCoder MIT true @@ -63,7 +63,7 @@ --> true - 5.3.0 + 5.4.0