From b676758aa27f7712e18c869743193ef1005221f1 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Wed, 23 Sep 2026 16:43:07 +0200 Subject: [PATCH 1/3] fix(SchemaChecker): silence reports from db:add-missing-indices - command is not mandatory to run, therefore checks should not be reported as blocking - example: - oc_mail_trusted_senders: missing index 'mail_trusted_senders_type' - oc_mail_trusted_senders: unexpected index 'mail_trusted_senders_idx' Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Maksim Sukharev --- core/Command/Db/CheckSchema.php | 2 +- lib/private/DB/SchemaChecker.php | 60 +++++++++++++++++++++++++++----- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/core/Command/Db/CheckSchema.php b/core/Command/Db/CheckSchema.php index d1183d22777be..2a5e6f3ce0c12 100644 --- a/core/Command/Db/CheckSchema.php +++ b/core/Command/Db/CheckSchema.php @@ -57,7 +57,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } /** - * @param array, app: ?string, enabled: bool}>> $byDisabledApp + * @param array, app: ?string, enabled: bool, optionalIndex: bool}>> $byDisabledApp */ private function printDisabledAppFindings(array $byDisabledApp, OutputInterface $output): void { if ($byDisabledApp === []) { diff --git a/lib/private/DB/SchemaChecker.php b/lib/private/DB/SchemaChecker.php index a2877cb056e11..f43bd7e58d8b1 100644 --- a/lib/private/DB/SchemaChecker.php +++ b/lib/private/DB/SchemaChecker.php @@ -16,6 +16,8 @@ use OC\Migration\NullOutput; use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; +use OCP\DB\Events\AddMissingIndicesEvent; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IAppConfig; /** @@ -28,11 +30,12 @@ public function __construct( private readonly Connection $connection, private readonly IAppConfig $appConfig, private readonly IAppManager $appManager, + private readonly IEventDispatcher $eventDispatcher, ) { } /** - * @return list, app: ?string, enabled: bool}> + * @return list, app: ?string, enabled: bool, optionalIndex: bool}> */ public function getFindings(?string $onlyTable = null): array { $expectedSchema = new Schema(); @@ -65,18 +68,21 @@ public function getFindings(?string $onlyTable = null): array { $comparator = $this->connection->createSchemaManager()->createComparator(); $diff = $comparator->compareSchemas($liveSchema, $expectedSchema); + $optionalIndexNames = $this->getOptionalIndexNames(); - return array_map(function (array $finding) use ($disabledAppTableOwners, $enabledApps): array { + return array_map(function (array $finding) use ($disabledAppTableOwners, $enabledApps, $optionalIndexNames): array { $app = $disabledAppTableOwners[$finding['table']] ?? null; $finding['app'] = $app; // Only tables owned by a disabled app are non-blocking. $finding['enabled'] = $app === null || $app === 'core' || isset($enabledApps[$app]); + $finding['optionalIndex'] = ($finding['type'] === 'missing_index' || $finding['type'] === 'unexpected_index') + && isset($optionalIndexNames[$finding['table']][$finding['name']]); return $finding; }, $this->buildFindings($diff)); } /** - * @param array{table: string, type: string, name?: string, changes?: list, app?: ?string, enabled?: bool} $finding + * @param array{table: string, type: string, name?: string, changes?: list, app?: ?string, enabled?: bool, optionalIndex?: bool} $finding */ public function formatFinding(array $finding): string { return match ($finding['type']) { @@ -92,23 +98,28 @@ public function formatFinding(array $finding): string { } /** - * Splits findings into blocking ones (from core or an enabled app) and - * non-blocking ones, grouped by the disabled app that owns them. + * Splits findings into blocking ones (from core or an enabled app), + * non-blocking ones grouped by the disabled app that owns them, and + * non-blocking optional-index findings (only relevant if occ + * db:add-missing-indices was never run for that table). * - * @param list, app: ?string, enabled: bool}> $findings - * @return array{blocking: list, app: ?string, enabled: bool}>, byDisabledApp: array, app: ?string, enabled: bool}>>} + * @param list, app: ?string, enabled: bool, optionalIndex: bool}> $findings + * @return array{blocking: list, app: ?string, enabled: bool, optionalIndex: bool}>, byDisabledApp: array, app: ?string, enabled: bool, optionalIndex: bool}>>, optionalIndices: list, app: ?string, enabled: bool, optionalIndex: bool}>} */ public function partitionFindings(array $findings): array { $blocking = []; $byDisabledApp = []; + $optionalIndices = []; foreach ($findings as $finding) { - if ($finding['enabled']) { + if ($finding['optionalIndex']) { + $optionalIndices[] = $finding; + } elseif ($finding['enabled']) { $blocking[] = $finding; } else { $byDisabledApp[$finding['app']][] = $finding; } } - return ['blocking' => $blocking, 'byDisabledApp' => $byDisabledApp]; + return ['blocking' => $blocking, 'byDisabledApp' => $byDisabledApp, 'optionalIndices' => $optionalIndices]; } private function applyMigrations(string $app, Schema $schema): void { @@ -214,6 +225,37 @@ private function materializeUniqueConstraints(Schema $schema): void { } } + /** + * Apps can register indices that are only ever created or renamed via + * occ db:add-missing-indices (AddMissingIndicesEvent), not through a + * versioned migration. Since running that command is optional, whether + * such an index exists on the live schema depends on whether an admin + * ever ran it - it is not itself a sign of drift in either direction. + * Collect their names here so findings about them can be reported + * separately instead of as blocking missing/unexpected index findings. + * + * @return array> table name => set of index names + */ + private function getOptionalIndexNames(): array { + $event = new AddMissingIndicesEvent(); + $this->eventDispatcher->dispatchTyped($event); + + $names = []; + foreach ($event->getMissingIndices() as $missingIndex) { + $table = $this->connection->getPrefix() . $missingIndex['tableName']; + $names[$table][$missingIndex['indexName']] = true; + } + foreach ($event->getIndicesToReplace() as $toReplace) { + $table = $this->connection->getPrefix() . $toReplace['tableName']; + $names[$table][$toReplace['newIndexName']] = true; + foreach ($toReplace['oldIndexNames'] as $oldIndexName) { + $names[$table][$oldIndexName] = true; + } + } + + return $names; + } + private function keepOnlyTable(Schema $schema, string $tableName): void { foreach ($schema->getTables() as $table) { if ($table->getName() !== $tableName) { From 5d3cab91d2ce7a3f6cad4569ff89be265c15d7c5 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Wed, 23 Sep 2026 17:06:38 +0200 Subject: [PATCH 2/3] fix(SchemaChecker): ignore literal defaults on TEXT/BLOB columns - MySQL and MariaDB silently drop a literal DEFAULT clause on TEXT/BLOB columns. Migrations that declare such a default are a false-positive findings. - example: - oc_flow_checks: column 'class' differs in: default Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Maksim Sukharev --- lib/private/DB/SchemaChecker.php | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/private/DB/SchemaChecker.php b/lib/private/DB/SchemaChecker.php index f43bd7e58d8b1..74ad140cc4429 100644 --- a/lib/private/DB/SchemaChecker.php +++ b/lib/private/DB/SchemaChecker.php @@ -12,6 +12,7 @@ use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaDiff; use Doctrine\DBAL\Schema\TableDiff; +use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Types; use OC\Migration\NullOutput; use OCP\App\AppPathNotFoundException; @@ -19,6 +20,7 @@ use OCP\DB\Events\AddMissingIndicesEvent; use OCP\EventDispatcher\IEventDispatcher; use OCP\IAppConfig; +use OCP\IDBConnection; /** * Compares the live database schema against the schema expected for the @@ -343,7 +345,7 @@ private function getChangedColumnProperties(ColumnDiff $columnDiff): array { if ($columnDiff->hasNotNullChanged()) { $changes[] = 'nullable'; } - if ($columnDiff->hasDefaultChanged()) { + if ($columnDiff->hasDefaultChanged() && !$this->isIgnorableTextDefaultDiff($columnDiff)) { $changes[] = 'default'; } if ($columnDiff->hasAutoIncrementChanged()) { @@ -361,4 +363,20 @@ private function getChangedColumnProperties(ColumnDiff $columnDiff): array { return $changes; } + + /** + * MySQL and MariaDB silently ignore a literal DEFAULT clause on TEXT and + * BLOB columns - only NULL is ever actually stored for them. A migration + * that declares such a default therefore always disagrees with the live + * schema on these platforms, even though nothing has actually drifted. + */ + private function isIgnorableTextDefaultDiff(ColumnDiff $columnDiff): bool { + if (!in_array($this->connection->getDatabaseProvider(), [IDBConnection::PLATFORM_MYSQL, IDBConnection::PLATFORM_MARIADB], true)) { + return false; + } + + $typeName = Type::getTypeRegistry()->lookupName($columnDiff->getNewColumn()->getType()); + + return in_array($typeName, [Types::TEXT, Types::BLOB], true); + } } From 208449f684a684c1da7f7e2098ca55511470fee4 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Wed, 23 Sep 2026 17:37:35 +0200 Subject: [PATCH 3/3] fix(SchemaChecker): normalize long STRING columns to TEXT - Migrator::getDiff() rewrites any STRING column longer than 4000 characters to TEXT for consistency - SchemaChecker replays the same migrations for expected schema, but without going through that rewrite, reporting false positive finding. - example: - oc_bookmarks: column 'url' differs in: type Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Maksim Sukharev --- lib/private/DB/SchemaChecker.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/private/DB/SchemaChecker.php b/lib/private/DB/SchemaChecker.php index 74ad140cc4429..51697cecf7a24 100644 --- a/lib/private/DB/SchemaChecker.php +++ b/lib/private/DB/SchemaChecker.php @@ -12,6 +12,7 @@ use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaDiff; use Doctrine\DBAL\Schema\TableDiff; +use Doctrine\DBAL\Types\StringType; use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Types\Types; use OC\Migration\NullOutput; @@ -60,6 +61,7 @@ public function getFindings(?string $onlyTable = null): array { $this->addMigrationsTable($expectedSchema); $this->materializeUniqueConstraints($expectedSchema); + $this->normalizeLongStringColumns($expectedSchema); $liveSchema = $this->connection->createSchema(); @@ -227,6 +229,25 @@ private function materializeUniqueConstraints(Schema $schema): void { } } + /** + * Migrator::getDiff() (see lib/private/DB/Migrator.php) rewrites any + * STRING column longer than 4000 characters to TEXT before it generates + * DDL, for consistency between the supported databases. That rewrite + * only happens when a migration is actually applied, never when it is + * replayed here to build the expected schema - so without repeating it, + * any such column would forever be reported as a type mismatch. + */ + private function normalizeLongStringColumns(Schema $schema): void { + foreach ($schema->getTables() as $table) { + foreach ($table->getColumns() as $column) { + if ($column->getType() instanceof StringType && $column->getLength() > 4000) { + $column->setType(Type::getType(Types::TEXT)); + $column->setLength(null); + } + } + } + } + /** * Apps can register indices that are only ever created or renamed via * occ db:add-missing-indices (AddMissingIndicesEvent), not through a