diff --git a/lib/Listener/AddMissingIndicesListener.php b/lib/Listener/AddMissingIndicesListener.php index 45dd95ae5d..40f5d2fbe6 100644 --- a/lib/Listener/AddMissingIndicesListener.php +++ b/lib/Listener/AddMissingIndicesListener.php @@ -10,11 +10,40 @@ use OCP\DB\Events\AddMissingIndicesEvent; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\IDBConnection; /** * @template-implements IEventListener */ final class AddMissingIndicesListener implements IEventListener { + /** + * Indices on the values of cell tables that store them as text, by table. Oracle stores these + * values as CLOB, which it cannot index, and MySQL indexes only a prefix of them. + */ + public const TEXT_VALUE_INDICES = [ + 'tables_row_cells_selection' => 'tables_row_cells_selection_cv', + 'tables_row_cells_datetime' => 'tables_row_cells_datetime_cv', + 'tables_row_cells_usergroup' => 'tables_row_cells_usergroup_cv', + ]; + + /** + * Covers single selection option ids, dates and user, group and team ids entirely; a + * multi-selection is indexed by its first option ids only. + */ + public const TEXT_VALUE_PREFIX_LENGTH = 64; + + /** + * Cell tables whose text values have no length limit, as a multi-selection keeps all its option + * ids in one value. PostgreSQL indexes the whole value and rejects index entries above 2704 + * bytes, so these indices are left out there. + */ + public const UNBOUNDED_TEXT_VALUE_TABLES = ['tables_row_cells_selection']; + + public function __construct( + private readonly IDBConnection $connection, + ) { + } + public function handle(Event $event): void { if (!$event instanceof AddMissingIndicesEvent) { return; @@ -23,5 +52,17 @@ public function handle(Event $event): void { $event->addMissingIndex('tables_columns', 'tables_columns_t_id', ['table_id']); $event->addMissingIndex('tables_row_sleeves', 'tables_row_sleeves_t_id', ['table_id']); $event->addMissingIndex('tables_tables', 'tables_tables_ownership', ['ownership']); + $event->addMissingIndex('tables_row_cells_number', 'tables_row_cells_number_cv', ['column_id', 'value']); + + $platform = $this->connection->getDatabaseProvider(); + if ($platform === IDBConnection::PLATFORM_ORACLE) { + return; + } + foreach (self::TEXT_VALUE_INDICES as $tableName => $indexName) { + if ($platform === IDBConnection::PLATFORM_POSTGRES && in_array($tableName, self::UNBOUNDED_TEXT_VALUE_TABLES, true)) { + continue; + } + $event->addMissingIndex($tableName, $indexName, ['column_id', 'value'], ['lengths' => [null, self::TEXT_VALUE_PREFIX_LENGTH]]); + } } } diff --git a/lib/Migration/Version000700Date20230916000000.php b/lib/Migration/Version000700Date20230916000000.php index 84a38a10bf..2e542b731e 100644 --- a/lib/Migration/Version000700Date20230916000000.php +++ b/lib/Migration/Version000700Date20230916000000.php @@ -11,13 +11,38 @@ namespace OCA\Tables\Migration; use Closure; +use Doctrine\DBAL\Schema\Table; use OCP\DB\Exception; use OCP\DB\ISchemaWrapper; +use OCP\DB\Schema\ITable; use OCP\DB\Types; +use OCP\IDBConnection; use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; class Version000700Date20230916000000 extends SimpleMigrationStep { + /** + * Indices on the values of cell tables, by cell type, as AddMissingIndicesListener adds them to + * existing installations. Text values cannot be indexed on Oracle, and only by prefix on MySQL. + */ + private const VALUE_INDICES = [ + 'number' => 'tables_row_cells_number_cv', + 'datetime' => 'tables_row_cells_datetime_cv', + 'selection' => 'tables_row_cells_selection_cv', + ]; + private const TEXT_VALUE_PREFIX_LENGTH = 64; + + /** + * Cell types whose text values have no length limit, as a multi-selection keeps all its option + * ids in one value. PostgreSQL indexes the whole value and rejects index entries above 2704 + * bytes, so their index is left out there. + */ + private const UNBOUNDED_TEXT_VALUE_TYPES = ['selection']; + + public function __construct( + private readonly IDBConnection $connection, + ) { + } /** * this is a copy from the definition set in OCA\Tables\Helper\ColumnsHelper with added types @@ -80,10 +105,30 @@ private function createRowValueTable(ISchemaWrapper $schema, string $name, strin $table->addColumn('last_edit_at', Types::DATETIME, ['notnull' => true]); $table->addColumn('last_edit_by', Types::STRING, ['notnull' => true, 'length' => 64]); $table->addIndex(['column_id', 'row_id']); + $this->addValueIndex($table, $name, $type); $table->setPrimaryKey(['id']); } } + private function addValueIndex(Table|ITable $table, string $name, string $type): void { + $indexName = self::VALUE_INDICES[$name] ?? null; + if ($indexName === null) { + return; + } + if ($type !== Types::TEXT) { + $table->addIndex(['column_id', 'value'], $indexName); + return; + } + $platform = $this->connection->getDatabaseProvider(); + if ($platform === IDBConnection::PLATFORM_ORACLE) { + return; + } + if ($platform === IDBConnection::PLATFORM_POSTGRES && in_array($name, self::UNBOUNDED_TEXT_VALUE_TYPES, true)) { + return; + } + $table->addIndex(['column_id', 'value'], $indexName, [], ['lengths' => [null, self::TEXT_VALUE_PREFIX_LENGTH]]); + } + private function createRowSleevesTable(ISchemaWrapper $schema) { if (!$schema->hasTable('tables_row_sleeves')) { $table = $schema->createTable('tables_row_sleeves'); diff --git a/lib/Migration/Version000800Date20240712000000.php b/lib/Migration/Version000800Date20240712000000.php index b3c4d8e2d2..31822e84eb 100644 --- a/lib/Migration/Version000800Date20240712000000.php +++ b/lib/Migration/Version000800Date20240712000000.php @@ -12,10 +12,23 @@ use Closure; use OCP\DB\ISchemaWrapper; use OCP\DB\Types; +use OCP\IDBConnection; use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; class Version000800Date20240712000000 extends SimpleMigrationStep { + /** + * As AddMissingIndicesListener adds it to existing installations. Text values cannot be indexed + * on Oracle, and only by prefix on MySQL. + */ + private const VALUE_INDEX = 'tables_row_cells_usergroup_cv'; + private const TEXT_VALUE_PREFIX_LENGTH = 64; + + public function __construct( + private readonly IDBConnection $connection, + ) { + } + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); @@ -40,6 +53,9 @@ private function createUserGroupTable(ISchemaWrapper $schema, string $name, stri $table->addColumn('last_edit_at', Types::DATETIME, ['notnull' => true]); $table->addColumn('last_edit_by', Types::STRING, ['notnull' => true, 'length' => 64]); $table->addIndex(['column_id', 'row_id']); + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $table->addIndex(['column_id', 'value'], self::VALUE_INDEX, [], ['lengths' => [null, self::TEXT_VALUE_PREFIX_LENGTH]]); + } $table->setPrimaryKey(['id']); return $schema; } diff --git a/tests/unit/Listener/AddMissingIndicesListenerTest.php b/tests/unit/Listener/AddMissingIndicesListenerTest.php new file mode 100644 index 0000000000..88285fb4aa --- /dev/null +++ b/tests/unit/Listener/AddMissingIndicesListenerTest.php @@ -0,0 +1,82 @@ + + */ + public static function platformsWithTextIndicesProvider(): array { + return [ + 'MySQL' => [IDBConnection::PLATFORM_MYSQL], + 'PostgreSQL' => [IDBConnection::PLATFORM_POSTGRES], + 'SQLite' => [IDBConnection::PLATFORM_SQLITE], + ]; + } + + /** + * @dataProvider platformsWithTextIndicesProvider + */ + public function testIndexesTheValuesOfEveryCellTableButText(string $platform): void { + $indices = $this->missingIndicesOn($platform); + + $this->assertSame('tables_row_cells_number', $indices['tables_row_cells_number_cv']['tableName']); + $this->assertSame(['column_id', 'value'], $indices['tables_row_cells_number_cv']['columns']); + $this->assertSame([], $indices['tables_row_cells_number_cv']['options']); + foreach (AddMissingIndicesListener::TEXT_VALUE_INDICES as $tableName => $indexName) { + if ($platform === IDBConnection::PLATFORM_POSTGRES && in_array($tableName, AddMissingIndicesListener::UNBOUNDED_TEXT_VALUE_TABLES, true)) { + continue; + } + $this->assertSame($tableName, $indices[$indexName]['tableName']); + $this->assertSame(['column_id', 'value'], $indices[$indexName]['columns']); + $this->assertSame(['lengths' => [null, AddMissingIndicesListener::TEXT_VALUE_PREFIX_LENGTH]], $indices[$indexName]['options']); + } + $this->assertNotContains('tables_row_cells_text', array_column($indices, 'tableName')); + } + + public function testLeavesOutTheSelectionValueIndexOnPostgreSql(): void { + $indices = $this->missingIndicesOn(IDBConnection::PLATFORM_POSTGRES); + + $this->assertArrayNotHasKey('tables_row_cells_selection_cv', $indices); + $this->assertArrayHasKey('tables_row_cells_datetime_cv', $indices); + $this->assertArrayHasKey('tables_row_cells_usergroup_cv', $indices); + } + + public function testLeavesOutTheTextValueIndicesOnOracle(): void { + $indices = $this->missingIndicesOn(IDBConnection::PLATFORM_ORACLE); + + $this->assertArrayHasKey('tables_row_cells_number_cv', $indices); + foreach (AddMissingIndicesListener::TEXT_VALUE_INDICES as $indexName) { + $this->assertArrayNotHasKey($indexName, $indices); + } + } + + /** + * @return array + */ + private function missingIndicesOn(string $platform): array { + $connection = $this->createMock(IDBConnection::class); + $connection->method('getDatabaseProvider')->willReturn($platform); + $event = new AddMissingIndicesEvent(); + + (new AddMissingIndicesListener($connection))->handle($event); + + $indices = []; + foreach ($event->getMissingIndices() as $index) { + $indices[$index['indexName']] = ['tableName' => $index['tableName'], 'columns' => $index['columns'], 'options' => $index['options']]; + } + return $indices; + } +} diff --git a/tests/unit/Migration/CellValueIndicesTest.php b/tests/unit/Migration/CellValueIndicesTest.php new file mode 100644 index 0000000000..a98d18177e --- /dev/null +++ b/tests/unit/Migration/CellValueIndicesTest.php @@ -0,0 +1,96 @@ + + */ + public static function platformsProvider(): array { + return [ + 'MySQL' => [IDBConnection::PLATFORM_MYSQL], + 'PostgreSQL' => [IDBConnection::PLATFORM_POSTGRES], + 'SQLite' => [IDBConnection::PLATFORM_SQLITE], + 'Oracle' => [IDBConnection::PLATFORM_ORACLE], + ]; + } + + /** + * @dataProvider platformsProvider + */ + public function testMigrationsCreateTheIndicesTheListenerAdds(string $platform): void { + $connection = $this->createMock(IDBConnection::class); + $connection->method('getDatabaseProvider')->willReturn($platform); + + $this->assertSame($this->valueIndicesFromListener($connection), $this->valueIndicesFromMigrations($connection)); + } + + /** + * @return array + */ + private function valueIndicesFromListener(IDBConnection $connection): array { + $event = new AddMissingIndicesEvent(); + (new AddMissingIndicesListener($connection))->handle($event); + + $indices = []; + foreach ($event->getMissingIndices() as $index) { + if (str_ends_with($index['indexName'], '_cv')) { + $indices[$index['indexName']] = ['tableName' => $index['tableName'], 'columns' => $index['columns'], 'options' => $index['options']]; + } + } + ksort($indices); + return $indices; + } + + /** + * @return array + */ + private function valueIndicesFromMigrations(IDBConnection $connection): array { + $indices = []; + $schema = $this->createMock(ISchemaWrapper::class); + $schema->method('hasTable')->willReturn(false); + $schema->method('createTable')->willReturnCallback(function (string $tableName) use (&$indices) { + $table = $this->createMock(interface_exists(ITable::class) ? ITable::class : Table::class); + $table->method('addIndex')->willReturnCallback( + function (array $columns, ?string $indexName = null, array $flags = [], array $options = []) use (&$indices, $tableName, $table) { + if ($indexName !== null && str_ends_with($indexName, '_cv')) { + $indices[$indexName] = ['tableName' => $tableName, 'columns' => $columns, 'options' => $options]; + } + return $table; + } + ); + return $table; + }); + $output = $this->createMock(IOutput::class); + + (new Version000700Date20230916000000($connection))->changeSchema($output, static fn () => $schema, []); + (new Version000800Date20240712000000($connection))->changeSchema($output, static fn () => $schema, []); + + ksort($indices); + return $indices; + } +}