# EF Core Removal Analysis — pgsql-jellyfin Fork

**Date**: 2025  
**Scope**: Full codebase analysis of whether EF Core can and should be removed from this PostgreSQL-only fork.  
**Verdict**: **Do not remove EF Core.** Address specific friction points instead.

---

## 1. Usage Inventory

EF Core (`IDbContextFactory<JellyfinDbContext>`) is injected into **82 call sites across ~35 source files**, permeating every layer:

| Layer | Files |
|---|---|
| API | `DatabaseViewsController` |
| Repository (Item) | `BaseItemRepository`, `PeopleRepository`, `MediaStreamRepository`, `MediaAttachmentRepository`, `KeyframeRepository`, `ChapterRepository` |
| Repository (Other) | `ActivityManager`, `DeviceManager`, `DisplayPreferencesManager`, `UserManager`, `TrickplayManager`, `MediaSegmentManager`, `AuthenticationManager`, `AuthorizationContext`, `LibraryOptionsRepository`, `BackupService` |
| Scheduled Tasks | `PeopleValidationTask`, `CleanupUserDataTask`, `CleanDatabaseScheduledTask`, `UserDataManager` |
| Migration Routines | 13 routines: `MigrateActivityLogDb`, `MigrateAuthenticationDb`, `MigrateDisplayPreferencesDb`, `MigrateKeyframeData`, `MigrateLibraryDb`, `MigrateLibraryUserData`, `MigrateRatingLevels`, `MoveExtractedFiles`, `RefreshCleanNames`, `RefreshInternalDateModified`, `FixDates`, `CleanMusicArtist`, (+ more) |
| Infrastructure | `JellyfinMigrationService`, `PostgresDatabaseProvider`, `DbContextFactoryHealthCheck`, `Program.cs`, `ServiceCollectionExtensions` |

The `JellyfinDbContext` itself defines **~35 DbSets** spanning 5 schemas (`activitylog`, `authentication`, `displaypreferences`, `library`, `users`).

---

## 2. Query Complexity — What EF Core Actually Does

### 2.1 BaseItemRepository.cs — The Core Case

`BaseItemRepository.cs` is **3,583 lines / 152 KB**, the largest file in the repository. It is the performance-critical code path for all library browsing, search, and playback-related queries. It is built entirely on EF Core LINQ.

**Key EF-dependent sub-systems inside it:**

| Method | EF Feature Used | Purpose |
|---|---|---|
| `TranslateQuery()` (lines 2548+, ~500 lines) | LINQ predicate composition over `IQueryable<BaseItemEntity>` | Translates `InternalItemsQuery` (60+ filter parameters) to SQL WHERE clauses |
| `ApplyOrder()` (~80 lines) | `OrderBy` / `ThenBy` over EF expression trees, `OrderMapper.MapOrderByField()` | Translates `ItemSortBy` enum to typed SQL ORDER BY |
| `ApplyNavigations()` | Conditional `Include()` / `ThenInclude()` | Eagerly loads TvExtras, AudioExtras, LiveTvExtras, Provider, Images, UserData per DTO field set |
| `ApplyGroupingFilter()` | `DistinctBy()` → PostgreSQL `DISTINCT ON` | Deduplication for presentation unique keys |
| `GetNextUpSeriesKeys()` | `Join()` + `GroupBy()` + `Max()` | Complex TV "next up" episode query |
| `GetItemValues()` | Nested `Join()` + `SelectMany()` + correlated subquery pattern | Artist/Genre/Studio item-value lookups |
| `UpdateOrInsertItemsAsync()` | `CreateExecutionStrategy().ExecuteAsync()`, `SaveChangesAsync()`, `Attach().State = EntityState.Modified`, `AddRange()` | Transactional batch upsert |
| Entity state management | `context.Entry(placeholder).State = EntityState.Detached` | Upsert conflict resolution |
| `EF.Constant()` | PostgreSQL-specific plan hint | Prevents parameterization of the placeholder UUID to allow index scans |

**LINQ operator counts in BaseItemRepository.cs** (approximate from code review):  
- `Where()`, `Select()`, `Join()`, `GroupBy()`, `Include()`, `ThenInclude()`, `OrderBy()`: **200+ call sites**  
- `ExecuteDelete()` / `ExecuteUpdate()` bulk ops: **~25 call sites**  
- `FromSqlRaw` / `FromSqlInterpolated`: **0** — reads are pure LINQ  
- `ExecuteSqlAsync()` raw SQL upserts: **~8** (ON CONFLICT upserts for extension tables)

The LINQ query composition is not superficial mapping sugar — `TranslateQuery` conditionally applies 30+ predicate branches (user data filters, rating filters, type filters, ancestor traversal, search term scoring, tag inheritance, etc.) based on `InternalItemsQuery` flags. This is the equivalent of a 500-line dynamic SQL builder, expressed in type-safe C#.

### 2.2 Other Repositories

The other repositories (`ActivityManager`, `UserManager`, `DeviceManager`, etc.) use EF more simply — primarily `Where()` + `Select()` + `SaveChanges()` patterns — but collectively represent hundreds more LINQ call sites.

### 2.3 Hybrid Usage Already Present

`BaseItemRepository.UpdateOrInsertItemsAsync()` already uses a hybrid approach: EF Core for reads and change-tracking, **raw `ExecuteSqlAsync()` for ON CONFLICT upserts**. This tells us the team is already reaching EF's limits on writes.

---

## 3. Migration Infrastructure

### 3.1 EF Schema Migrations Are DISABLED

Per `JellyfinMigrationService.cs` (line ~275):
```csharp
// DISABLED for .NET 11 preview: EF Core database migrations don't work with preview SDK
// Database schema is initialized from SQL scripts in PostgresDatabaseProvider instead
/*
if (stage is JellyfinMigrationStageTypes.CoreInitialisation)
{
    pendingDatabaseMigrations = migrationsAssembly.Migrations...
}
*/
```

**EF's primary justification — schema migration management — is already bypassed.** Schema is initialized via `sql/schema_init/create_database_schema.sql` + `psql`.

### 3.2 EF Is Still Used for Code Migration Tracking

`JellyfinMigrationService` still uses EF to track *which code routines have run* via `__EFMigrationsHistory`:

```csharp
var historyRepository = dbContext.GetService<IHistoryRepository>();
await historyRepository.CreateIfNotExistsAsync();
var appliedMigrations = await dbContext.Database.GetAppliedMigrationsAsync();
await dbContext.Database.ExecuteSqlRawAsync(
    historyRepository.GetInsertScript(new HistoryRow(migrationId, version)));
```

This usage of `IHistoryRepository` is what required the `SnakeCaseHistoryRepository` workaround (custom class overriding `MigrationIdColumnName` and `ProductVersionColumnName`, using `#pragma warning disable EF1001` because the base class is internal EF infrastructure).

---

## 4. Known Friction Points

These are the specific problems that prompted this analysis:

### 4.1 Snake_case Naming Impedance (FIXED, but fragile)

EF Core defaults to PascalCase. This fork uses snake_case PostgreSQL conventions. Three workarounds are required:

1. **`SnakeCaseNamingConvention`** — `IPropertyAddedConvention` that converts all property names to snake_case at model build time  
2. **`SnakeCaseHistoryRepository`** — custom `NpgsqlHistoryRepository` subclass overriding column name properties; requires `#pragma warning disable EF1001` (breaking internal EF API)  
3. **Manual table names in model configurations** — 31 `IEntityTypeConfiguration` files; two (`LibraryOptionsEntityConfiguration`, `HomeSectionConfiguration`) had PascalCase table names that overrode the convention and caused `relation "library.LibraryOptions" does not exist` errors at runtime

**Risk**: Every future upstream EF Core upgrade may break `SnakeCaseHistoryRepository` since it depends on internal EF infrastructure.

### 4.2 Execution Strategy Transaction Friction (FIXED, but imposed on all writers)

`EnableRetryOnFailure` in the PostgreSQL retry strategy makes EF Core incompatible with user-initiated `BeginTransaction()`. All transactional code must wrap in `context.Database.CreateExecutionStrategy().Execute(...)`. This has already been fixed in `MigrateRatingLevels` and `MigrateKeyframeData` but is an ongoing constraint.

### 4.3 EF-PostgreSQL Translation Gaps

Multiple code comments document EF Core translation failures:

- `DistinctBy()` on complex types sometimes cannot be translated → fallback to `ApplyGroupingInMemory()` using **reflection on anonymous types**  
- `GetItemValues()` in some paths falls back to `.AsEnumerable()` for in-memory filtering  
- `GetLatestItemList()` had to be split into two queries to avoid untranslatable nested `Min()` subqueries  
- A documented 165s query regression was caused by EF generating correlated subqueries — fixed by switching to `DistinctBy()` → `DISTINCT ON`  
- `AsSingleQuery()` required explicitly to prevent N+1 for multi-table includes

### 4.4 Inconsistent Table Naming in Raw SQL

`UpdateOrInsertItemsAsync()` uses raw `ExecuteSqlAsync()` with PascalCase quoted names:

```csharp
$@"INSERT INTO library.""BaseItemProviders"" (""ItemId"", ""ProviderId"", ""ProviderValue"")..."
$@"INSERT INTO library.""BaseItemTvExtras"" (""ItemId"", ..."
```

These PascalCase quoted names (`"BaseItemProviders"`, `"BaseItemTvExtras"`) are inconsistent with the snake_case goal of the fork. The actual table names in PostgreSQL must match these quoted strings exactly — implying the schema init SQL creates them with PascalCase quoted names, not the snake_case convention applied to EF properties.

### 4.5 Known Performance Anti-Pattern in `GetItemValues`

```csharp
// TODO: This is bad refactor!
itemCount = new ItemCounts() {
    SeriesCount = itemCountQuery!.Count(f => f.Type == seriesTypeName),
    EpisodeCount = itemCountQuery!.Count(f => f.Type == episodeTypeName),
    // ... 5 more COUNT queries
}
```

This executes 7 separate `COUNT` queries per item in a result set — a classic N+1 problem embedded in an EF projection. This is an EF translation limitation (aggregate counts in a nested `SELECT` project).

---

## 5. Can EF Core Be Removed?

**Technically: Yes. Practically: The cost is prohibitive.**

A full removal would require:

1. **Rewriting `BaseItemRepository.cs`** (3,583 lines) as raw SQL — either Dapper or direct `NpgsqlCommand`. The `TranslateQuery` method alone is a ~500-line dynamic query builder that would need to be rewritten as a SQL string builder. All `Include()` chains would become explicit JOINs. All `ExecuteUpdate`/`ExecuteDelete` calls would become parameterized SQL.

2. **Replacing 31 ModelConfiguration files** with SQL schema definitions (partially done via `sql/schema_init/`).

3. **Replacing the migration tracking system** (`IHistoryRepository`) with a simple SQL table + custom tracker.

4. **Rewriting all other repositories** (~15 files) as Dapper or raw Npgsql.

5. **Replacing all `IDbContextFactory` injection points** (~35 files, 82 sites) with new connection/command factories.

**Estimated effort: 3–6 person-months of careful rewrite, plus extensive testing coverage.**

Alternatives assessed:

| Alternative | Pros | Cons |
|---|---|---|
| **Full removal (Dapper + Npgsql)** | Full control of SQL; no EF friction; better PostgreSQL-native features | 3-6 month rewrite; no dynamic LINQ query composition; lose `ExecuteUpdate`/`ExecuteDelete`; high regression risk |
| **Hybrid (EF reads, Dapper writes)** | Eliminates write friction; keeps LINQ query composition | Two persistence paradigms; still need EF for reads; does not eliminate snake_case friction |
| **Keep EF, address friction points** | Minimal disruption; preserves LINQ query composition | Ongoing EF+PostgreSQL compatibility work; preview SDK risk |
| **Replace only migration tracking** | Eliminates `SnakeCaseHistoryRepository` hack | Small gain; does not address execution strategy or translation friction |

---

## 6. Should EF Core Be Removed?

**No — but specific friction points should be addressed deliberately.**

### 6.1 Why EF Core Should Stay

**The LINQ query composition in `TranslateQuery` / `ApplyOrder` is EF Core's core contribution to this codebase.** It provides:

- **Type safety** across 60+ filter parameters in `InternalItemsQuery`
- **Composability** — predicates are built incrementally, each branch independently testable
- **Maintainability** — 500 lines of C# expression trees is significantly more maintainable than equivalent raw SQL string building
- **`ExecuteUpdate`/`ExecuteDelete`** — these EF 7+ bulk operations map directly to `UPDATE ... WHERE` and `DELETE ... WHERE` without loading entities; replacing these with raw SQL would produce functionally identical code with more boilerplate
- **Navigation property loading** (`Include`) — the conditional navigation loading in `ApplyNavigations` maps cleanly to the DTO field selection pattern used by the API layer

EF Core's value in this codebase is almost entirely in the **read path** (LINQ-to-SQL translation) and **bulk operations** (`ExecuteUpdate`/`ExecuteDelete`). Its weaknesses are in the write path and infrastructure (migration history, naming conventions, execution strategy).

### 6.2 Recommended Targeted Actions

In priority order:

#### Priority 1 — Replace Migration History Tracking (Low effort, high safety gain)

The `SnakeCaseHistoryRepository` workaround depends on EF internal APIs (`#pragma warning disable EF1001`). Replace the code migration tracking with a simple dedicated table and Dapper/Npgsql:

```sql
CREATE TABLE IF NOT EXISTS public.jellyfin_applied_migrations (
    migration_id VARCHAR(150) PRIMARY KEY,
    applied_at TIMESTAMPTZ NOT NULL DEFAULT now(),
    product_version VARCHAR(32) NOT NULL
);
```

Remove `JellyfinMigrationService`'s dependency on `IHistoryRepository` and `GetAppliedMigrationsAsync()`. This eliminates the most fragile EF workaround.

#### Priority 2 — Fix Raw SQL Table Names to Match snake_case Schema (Low effort, correctness)

`UpdateOrInsertItemsAsync` uses `library."BaseItemProviders"`, `library."BaseItemTvExtras"`, etc. If the fork's goal is snake_case naming, verify whether these tables exist as quoted PascalCase or as snake_case in PostgreSQL, and make the raw SQL consistent. Quoted PascalCase in PostgreSQL is case-sensitive and is a footgun.

#### Priority 3 — Replace Write Path with Raw SQL/Dapper (Medium effort, eliminates execution strategy friction)

The `UpdateOrInsertItemsAsync` method is already a hybrid (raw SQL for ON CONFLICT upserts + EF for the surrounding logic). Converting the remaining EF write operations (`SaveChangesAsync`, `Attach().State = EntityState.Modified`, `AddRange()`) in this method to direct Npgsql/Dapper calls would:

- Eliminate the `CreateExecutionStrategy().Execute()` wrappers
- Enable direct PostgreSQL `ON CONFLICT DO UPDATE` throughout
- Remove the impedance between EF change tracking and PostgreSQL's native upsert semantics

**Do not extend this to reads** — the LINQ query composition is worth keeping.

#### Priority 4 — Fix the N+1 in `GetItemValues` (Medium effort, performance)

The 7 separate `COUNT` queries per item should be replaced with a single `GROUP BY type, COUNT(*)` query executed once and pivoted in memory. This is a correctness/performance issue independent of whether EF is removed.

---

## 7. Summary

| Question | Answer |
|---|---|
| **Can EF Core be removed?** | Yes, technically. Estimated 3–6 person-months of work. |
| **Should EF Core be removed?** | No. The LINQ query composition in `BaseItemRepository` is EF's core value and cost-prohibitive to replace. |
| **Is EF Core being used optimally?** | No. The write path, migration tracking, and naming convention workarounds are sources of ongoing friction. |
| **What should be done instead?** | Replace migration tracking with a native SQL table; convert the write path in `BaseItemRepository` to raw SQL; keep EF for the LINQ read path. |
| **Biggest ongoing risk?** | `SnakeCaseHistoryRepository` uses internal EF APIs (`EF1001`) that may break on EF Core version bumps. Replacing migration tracking eliminates this. |
| **Second biggest risk?** | EF Core + Npgsql preview packages (`11.0.0-preview.1`) on `net11.0`. Schema migrations are already disabled because of this. Pin to stable as soon as net11.0 releases. |