| name | ef-core-sqlserver |
| description | Database best-practices review for EF Core + SQL Server (Microsoft.Data.SqlClient). Use when reviewing entities, mappings, migrations, queries, and transactions that hit SQL Server — checks correct modeling and types (datetime2, decimal, uniqueidentifier, json), PK/clustered-index choice, indexes (including FKs, which SQL Server doesn't index on its own), safe/zero-downtime migrations, query performance (N+1, projection, AsNoTracking, implicit conversion, parameter sniffing), optimistic concurrency with rowversion, and data security. |
EF Core + SQL Server — Database Best Practices
Review focused on the data layer with EF Core and SQL Server (Microsoft.Data.SqlClient driver). Act as a senior DBA/engineer: be direct, technical, and skip the praise.
Reference: EF Core 10 (LTS) and SQL Server 2025 are the current baseline. Features tagged with a version/edition (native json type, ONLINE = ON, etc.) depend on the environment — flag it when the target is an earlier version/edition.
Scope
Review ONLY what changed in the diff — entities, IEntityTypeConfiguration, DbContext, migrations, LINQ and SQL queries. Assess against the practices below.
Modeling & Schema
- Correct types:
datetime2 (not datetime — lower precision and range) or datetimeoffset for timezone; decimal/numeric with defined precision for money (never float/real); nvarchar/varchar with an adequate size — avoid nvarchar(max) when a bounded size works (it can't be an index key and goes off-row); uniqueidentifier for Guid; text/ntext are deprecated (use varchar(max)/nvarchar(max)).
- Constraints in the database: invariants enforced in the schema —
NOT NULL, UNIQUE, FK, and CHECK — not just in the application. The database is the last line of defense for integrity.
- PK and clustered index: SQL Server creates a clustered index on the PK by default; the key should be narrow, static, and increasing (
int/bigint IDENTITY). A random Guid (NEWID()) as a clustered PK fragments and causes page splits — use NEWSEQUENTIALID() or keep the Guid as a non-clustered PK with another clustered key.
- Narrow clustered key: every non-clustered index carries the clustered key as a lookup — a wide PK inflates all indexes and storage.
- Explicit FK behavior:
OnDelete set deliberately (Restrict/Cascade/SetNull); watch for multiple/cyclic cascade paths — SQL Server rejects them and the migration fails.
- JSON as structured data: model via a complex type with
ToJson() (prefer complex types to owned entities). On SQL Server 2025 (compat level 170+) EF Core 10 maps to the native json type — more efficient and safer than text; on earlier versions it falls back to nvarchar(max). Watch the migration: when upgrading to EF10 with compat 170+, existing JSON columns in nvarchar(max) are converted to json on the first migration — review/consent (or pin nvarchar(max) to opt out).
Indexes
- Index the FKs: SQL Server does not create an automatic index on FK columns (only on the PK and on
UNIQUE) — without it, JOINs and deletes on the parent table scan.
- Indexes by access pattern: columns used in
WHERE/JOIN/ORDER BY; in a composite index, column order follows selectivity and the query.
- Covering and filtered:
INCLUDE to cover the query and avoid key lookups; filtered indexes (WHERE) for subsets; don't over-index (each index costs on writes).
- Consult the optimizer: use missing-index DMVs / the execution plan to justify indexes instead of guessing.
Migrations
- Reviewed and reversible: check the generated SQL (don't blindly trust scaffolding) and have a working
Down.
- No
Migrate() on startup in production: apply migrations via a controlled pipeline/script — Database.Migrate() at boot causes races and locks with multiple instances.
- Zero-downtime (expand & contract): changes compatible with the old app version during deploy — add first, migrate data, remove later; never rename/drop a column in use.
- Mind the locks: schema changes take a
Sch-M lock and block the table; use CREATE INDEX ... WITH (ONLINE = ON) (Enterprise/Azure SQL) to build an index without blocking; adding a NOT NULL column with a constant default is usually metadata-only on SQL Server 2012+.
Queries & Performance
AsNoTracking on reads: read-only queries shouldn't pay the change-tracker cost.- Projection with
Select: fetch only the needed columns instead of materializing the whole entity (avoids SELECT *).
- No N+1: load relationships with
Include/projection; accidental lazy loading in a loop is the classic antipattern.
AsSplitQuery: for multiple collection Includes, avoids the cartesian explosion of a single JOIN — always with a deterministic OrderBy (EF Core 10 now guarantees consistent ordering across the queries, but the stable key is the model's responsibility).- Stable pagination: deterministic
OrderBy with Skip/Take (OFFSET/FETCH); for large offsets, prefer keyset/seek pagination.
- Batch operations:
ExecuteUpdate/ExecuteDelete (EF Core 7+) instead of loading entities and SaveChanges in a loop; in EF Core 10 ExecuteUpdateAsync accepts a regular lambda (no longer an expression tree) and reaches properties inside JSON columns.
DbContext pooling: AddDbContextPool in high-throughput APIs; DbContext is short-lived and not thread-safe.- Implicit conversion: a
varchar column compared with an nvarchar parameter (EF's default is Unicode) generates CONVERT_IMPLICIT, makes the predicate non-sargable, and ignores the index — align the mapping (IsUnicode(false)) or the column.
- Parameter sniffing: bad plans with skewed data; consider
OPTION (RECOMPILE) or local variables in critical queries.
CancellationToken: propagated in every async query down to the database.
Transactions & Concurrency
- Optimistic concurrency: use a
rowversion column (IsRowVersion()) as a token to detect concurrent writes.
- Lean transactions:
SaveChanges is already atomic; open an explicit transaction only when multiple operations must be atomic — and never hold the transaction open during external I/O (HTTP/queue).
- Locking and isolation: consider
READ_COMMITTED_SNAPSHOT (RCSI) to reduce reader/writer blocking; deliberate isolation level; consistent access order across transactions to avoid deadlocks.
- Connection resilience:
EnableRetryOnFailure (SqlServerRetryingExecutionStrategy) for transient failures, especially on Azure SQL — aware that, with user-initiated transactions, retry requires an explicit execution block.
Connection & Security
- Driver and pooling:
Microsoft.EntityFrameworkCore.SqlServer over Microsoft.Data.SqlClient (not the legacy System.Data.SqlClient); pooling (on by default) with Max Pool Size and Command Timeout sized. Encrypt=True is the default since SqlClient 4.0 — use a valid certificate instead of masking with TrustServerCertificate=True.
- Parameterized SQL:
FromSql/FromSqlInterpolated/parameters, never FromSqlRaw with a concatenated string (SQL injection) — EF Core 10 has an analyzer that warns about concatenation in raw SQL; credentials in a secret manager, not in the repo.
- Logs without leaks:
EnableSensitiveDataLogging only in development; EF Core 10 already redacts inlined constants in the log by default (?), but sensitive parameters/values must not be logged in production.
Integrity & Filters
- Global filters: soft delete and multitenancy via
HasQueryFilter instead of repeated WHERE; EF Core 10 supports named filters (several per entity, individually disableable with IgnoreQueryFilters(["..."])). Careful when combining with Include/cascade.
Output format
Ignore what is correct — list only what needs to change, ordered by impact:
| # | Severity | Category | Comment | Rationale |
|---|
| 1 | 🔴 Critical | Modeling | PK uniqueidentifier defaulting to NEWID() on Orders | Random clustered index: massive fragmentation and page splits |
| 2 | 🟠 High | Indexes | FK CustomerId without an index | SQL Server doesn't index FKs; JOIN and parent delete scan |
| 3 | 🟡 Medium | Performance | Filter on a varchar column with an nvarchar parameter | CONVERT_IMPLICIT makes the predicate non-sargable, ignores the index |
| … | … | … | … | … |
Severities: 🔴 Critical (data loss/corruption, production lock, injection), 🟠 High (performance/index/concurrency), 🟡 Medium (best practice/maintainability), 🔵 Low (style/suggestion).
