Fix JSON deleted column index migrations

[bbrands02]

Summary

• Skip regular btree index creation for JSON columns in the faceting performance migration.
• Skip composite indexes that include JSON columns, including the deleted lifecycle indexes.
• Prevent the later duplicate objects_deleted_idx migration from indexing openregister_objects.deleted when it is JSON.

Root Cause

openregister_objects.deleted is created as Types::JSON in Version1Date20250321061615.php, but later migrations attempted to create normal btree indexes on that column. PostgreSQL rejects this with data type json has no default operator class for access method "btree".

Impact

This prevents upgrades, from failing on PostgreSQL when the migration chain reaches the deleted-column indexes. The guard also keeps MariaDB/MySQL on the safer path by avoiding plain indexes on JSON columns.

Validation

• php -l lib/Migration/Version1Date20250828120000.php
• php -l lib/Migration/Version1Date20250902140000.php
• git diff --check

[WilcoLouwerse]

🟢 Nit (non-blocking) — this inline JSON-type check duplicates the canUseBtreeIndex() helper added in Version1Date20250828120000.php. Since each Nextcloud migration is conventionally self-contained, this is fine to leave as-is, but if a third migration ever needs the same guard, consider lifting the check into a small shared helper (e.g. a trait under lib/Migration/) so the JSON-type rule lives in one place. Not blocking the merge.

[WilcoLouwerse]

No blockers. Fix correctly resolves the documented PostgreSQL btree-on-JSON failure for the deleted column; composite-index path also covered. One non-blocking nit on helper duplication: Nit (non-blocking). PR is mergeable=CONFLICTING — base merge to resolve before merge.

[MWest2020]

Review — Fix JSON deleted column index migrations

Strakke, gefocuste hotfix. Diff is klein (38/-1 op 2 files), de root-cause is correct geïdentificeerd (data type json has no default operator class for access method "btree" op PG voor Types::JSON-kolommen), en de aanpak (skip met info-log) is conservatief. CI groen op de relevante gates (lint, CodeQL, branch-protection). Target main is passend voor een hotfix.

🟡 Suggesties — geen blockers

1. Typing van $table in canUseBtreeIndex() (lib/Migration/Version1Date20250828120000.php:165):

private function canUseBtreeIndex($table, string $column): bool

$table is feitelijk een \Doctrine\DBAL\Schema\Table. PHPStan/Psalm zullen dit als mixed zien en geen autocompletion bieden. Voor toekomstige editors van deze migration is een expliciete type-hint nuttig:

private function canUseBtreeIndex(\Doctrine\DBAL\Schema\Table $table, string $column): bool

2. Composite-index control-flow is leesbaar verbeterbaar (Version1Date20250828120000.php:128–135):

if ($this->canUseBtreeIndex($table, $column) === false) {
    $allColumnsExist = false;
    $output->info("Skipped composite index {$indexName} because {$column} is a JSON column");
    break;
}

$allColumnsExist = false is misleidend — de kolom bestaat wél, maar is JSON. Resultaat is correct, maar een latere lezer zal denken dat de kolom ontbreekt. Een aparte vlag (b.v. $hasJsonColumn = true; break;) of een direct continue op de buitenste loop zou de intentie expliciter maken.

3. Inconsistente extractie tussen de twee files: In Version1Date20250828120000.php is canUseBtreeIndex() netjes als helper geëxtraheerd, in Version1Date20250902140000.php staat dezelfde JSON-check inline:

$table->getColumn('deleted')->getType()->getName() === Types::JSON

Migrations delen geen helpers (terecht — elk is een one-shot), dus dit kán zo, maar als de stijl binnen deze PR consistent is, is dat sterker als signaal voor latere migrations.

🟡 Follow-up zorgen (niet voor deze PR)

4. Bestaande half-gemigreerde installs: De migrations Version1Date20250828120000 en Version1Date20250902140000 zijn van Aug/Sep 2025 — ze hebben dus al ~8 maanden gefaald op PostgreSQL-installs die op de getroffen versie waren. Die installs hebben mogelijk een deel van de indexes gekregen voordat de JSON-failure het hele migration-block aborteerde. Deze hotfix voorkomt nieuwe failures, maar doet niets aan reeds-aangebroken state. Is er een rescue-migration of occ-command nodig om bestaande installs in een consistente staat te brengen? Graag bevestigen of dat in een vervolg-PR komt.

5. Soft-delete filtering blijft traag op JSON: Door de skip krijgt PG géén index meer op deleted. Queries die soft-delete filteren (WHERE deleted IS NULL of WHERE deleted->>'deletedAt' IS NULL) blijven seq-scan op grote tabellen. Twee betere paden voor follow-up:

• Partial btree op een uitgepakte expressie: CREATE INDEX ON openregister_objects ((deleted IS NULL)) of op (deleted->>'deletedAt').
• Separate boolean kolom is_deleted die mee wordt bijgehouden — eenvoudiger te indexeren en cross-DB compatibel.

Niet voor deze hotfix-PR, wel relevant als perf-issue.

6. Database-keuze-validatie: canUseBtreeIndex() checkt op Types::JSON ongeacht het backend platform. Op MariaDB/MySQL kun je sinds 8.0 wél een functional index maken op een JSON-extractie. De huidige check is daarmee conservatief (ook MariaDB krijgt geen index), wat veilig is maar mogelijk MariaDB-installs onnodig benadeelt. Voor nu prima — een platform-aware variant kan later.

🟢 Wat goed gedaan is

• Duidelijke info-log per skip (zowel single als composite path) — geeft beheerders direct zicht op wat er gebeurt.
• Tweede migration heeft expliciete branches (JSON, niet-bestaand, normaal) in plaats van één if-statement — correcter dan een impliciete fall-through.
• Test plan documenteert PHP-lint + git diff --check. Voor een hotfix-met-runtime-impact had ik graag een phpunit-test toegevoegd zien die de migration in-memory tegen schemas met JSON- en non-JSON-deleted runt; niet kritiek voor merge, wel nice-to-have voor regressie.

Verdict

Lijkt me veilig om te mergen. De suggesties hierboven zijn small wins die in een aparte PR opgepakt kunnen worden, maar geen blockers voor deze hotfix.