From fd4dca6a37bce9ce7af22610a36b1d14af8e0f60 Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Tue, 3 Nov 2026 12:25:23 +0100 Subject: [PATCH 1/3] perf(rows): index cell values by column for number, selection, datetime and usergroup Filters compared every value of a column, because the cell tables only had a (column_id, row_id) index. The number, selection, datetime and usergroup cell tables now get a (column_id, value) index: through occ db:add-missing-indices on existing installations, and through the migrations that create these tables on new ones. - selection, datetime and usergroup values are text: MySQL and MariaDB index their first 64 characters, which covers option ids, dates and user, group and team ids; Oracle stores them as CLOB and cannot index them, so they are left out there - text cells stay unindexed, as long texts cannot be indexed on every database - the existing (column_id, row_id) indices are left as they are Refs #3092 Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Robert Zondervan --- lib/Listener/AddMissingIndicesListener.php | 29 ++++++++ .../Version000700Date20230916000000.php | 33 +++++++++ .../Version000800Date20240712000000.php | 16 +++++ .../AddMissingIndicesListenerTest.php | 70 +++++++++++++++++++ 4 files changed, 148 insertions(+) create mode 100644 tests/unit/Listener/AddMissingIndicesListenerTest.php diff --git a/lib/Listener/AddMissingIndicesListener.php b/lib/Listener/AddMissingIndicesListener.php index 45dd95ae5d..a03e0b6555 100644 --- a/lib/Listener/AddMissingIndicesListener.php +++ b/lib/Listener/AddMissingIndicesListener.php @@ -10,11 +10,32 @@ 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 selection option ids, dates and user, group and team ids entirely. + */ + public const TEXT_VALUE_PREFIX_LENGTH = 64; + + public function __construct( + private readonly IDBConnection $connection, + ) { + } + public function handle(Event $event): void { if (!$event instanceof AddMissingIndicesEvent) { return; @@ -23,5 +44,13 @@ 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']); + + if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { + return; + } + foreach (self::TEXT_VALUE_INDICES as $tableName => $indexName) { + $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..fcf18d567e 100644 --- a/lib/Migration/Version000700Date20230916000000.php +++ b/lib/Migration/Version000700Date20230916000000.php @@ -11,13 +11,31 @@ 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; + + 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 +98,25 @@ 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; + } + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $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..bbe9f281f6 --- /dev/null +++ b/tests/unit/Listener/AddMissingIndicesListenerTest.php @@ -0,0 +1,70 @@ + + */ + 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(['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) { + $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 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; + } +} From 9fe3687df729d80e62afac1853049c40df519575 Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Fri, 6 Nov 2026 16:51:55 +0100 Subject: [PATCH 2/3] fix(rows): leave the selection value index out on PostgreSQL A multi-selection keeps all its option ids in one value, which has no length limit. MySQL and MariaDB index only its first 64 characters, but PostgreSQL indexes the whole value and rejects index entries above 2704 bytes: with the index, saving a multi-selection of about 1,000 options failed with "index row size exceeds btree version 4 maximum". The selection value index is therefore not added on PostgreSQL, the same way the text value indices are not added on Oracle; in the benchmark it gave no measurable gain on PostgreSQL. - the listener and Version000700Date20230916000000 both leave it out - the prefix length docblock now says a multi-selection is indexed by its first option ids only - the listener test pins the table of the number index and covers the PostgreSQL case Refs #3092 Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Robert Zondervan --- lib/Listener/AddMissingIndicesListener.php | 16 ++++++++++++++-- .../Version000700Date20230916000000.php | 16 ++++++++++++++-- .../Listener/AddMissingIndicesListenerTest.php | 12 ++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/lib/Listener/AddMissingIndicesListener.php b/lib/Listener/AddMissingIndicesListener.php index a03e0b6555..40f5d2fbe6 100644 --- a/lib/Listener/AddMissingIndicesListener.php +++ b/lib/Listener/AddMissingIndicesListener.php @@ -27,10 +27,18 @@ final class AddMissingIndicesListener implements IEventListener { ]; /** - * Covers selection option ids, dates and user, group and team ids entirely. + * 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, ) { @@ -46,10 +54,14 @@ public function handle(Event $event): void { $event->addMissingIndex('tables_tables', 'tables_tables_ownership', ['ownership']); $event->addMissingIndex('tables_row_cells_number', 'tables_row_cells_number_cv', ['column_id', 'value']); - if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { + $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 fcf18d567e..2e542b731e 100644 --- a/lib/Migration/Version000700Date20230916000000.php +++ b/lib/Migration/Version000700Date20230916000000.php @@ -32,6 +32,13 @@ class Version000700Date20230916000000 extends SimpleMigrationStep { ]; 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, ) { @@ -112,9 +119,14 @@ private function addValueIndex(Table|ITable $table, string $name, string $type): $table->addIndex(['column_id', 'value'], $indexName); return; } - if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { - $table->addIndex(['column_id', 'value'], $indexName, [], ['lengths' => [null, self::TEXT_VALUE_PREFIX_LENGTH]]); + $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) { diff --git a/tests/unit/Listener/AddMissingIndicesListenerTest.php b/tests/unit/Listener/AddMissingIndicesListenerTest.php index bbe9f281f6..88285fb4aa 100644 --- a/tests/unit/Listener/AddMissingIndicesListenerTest.php +++ b/tests/unit/Listener/AddMissingIndicesListenerTest.php @@ -32,9 +32,13 @@ public static function platformsWithTextIndicesProvider(): array { 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']); @@ -42,6 +46,14 @@ public function testIndexesTheValuesOfEveryCellTableButText(string $platform): v $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); From 73014e0eaf77c404607009e8059775d30415306d Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Fri, 6 Nov 2026 16:51:55 +0100 Subject: [PATCH 3/3] test(rows): check the migrations create the value indices the listener adds Fresh installations get the cell value indices from the migrations that create the cell tables, existing installations from AddMissingIndicesListener. A test now runs both migrations against a mocked schema on MySQL, PostgreSQL, SQLite and Oracle and compares the value indices they create with the ones the listener adds, so a name or definition that drifts in one place fails instead of giving fresh installations a second index. Refs #3092 Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Robert Zondervan --- tests/unit/Migration/CellValueIndicesTest.php | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 tests/unit/Migration/CellValueIndicesTest.php 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; + } +}