From 68e70fe9900427d19d5153931c54886a25a7bc56 Mon Sep 17 00:00:00 2001 From: James Manuel Date: Wed, 30 Sep 2026 12:57:15 +0200 Subject: [PATCH 1/2] =?UTF-8?q?test:=20=E2=9C=85=20assert=20in-place=20str?= =?UTF-8?q?ing=20to=20text=20change=20fails=20on=20Oracle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds testChangeStringToTextEmptyTableFailsOnOracle to MigratorTest: an in-place string to text migration on an empty Oracle table raises ORA-22858. Skipped on other platforms. Co-Authored-By: Claude Opus 5.5 Signed-off-by: James Manuel Assisted-by: ClaudeCode:claude-opus-5-5 --- tests/lib/DB/MigratorTest.php | 39 +++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index 099419884e072..03b485b085ef3 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -244,6 +244,45 @@ public function testColumnCommentsInUpdate(): void { $this->addToAssertionCount(1); } + /** + * The string to text conversion fails on Oracle even with zero rows: + * ORA-22858 does not depend on the column's contents, so emptying the + * table is not a workaround. + */ + public function testChangeStringToTextEmptyTableFailsOnOracle(): void { + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $this->markTestSkipped('Documents an Oracle-specific limitation'); + } + + $startSchema = new Schema([], [], $this->getSchemaConfig()); + $table = $startSchema->createTable($this->tableName); + $table->addColumn('id', Types::BIGINT); + $table->addColumn('argument', Types::STRING, [ + 'notnull' => true, + 'length' => 4000, + ]); + + $endSchema = new Schema([], [], $this->getSchemaConfig()); + $table = $endSchema->createTable($this->tableName); + $table->addColumn('id', Types::BIGINT); + $table->addColumn('argument', Types::TEXT, [ + 'notnull' => true, + ]); + + $migrator = $this->getMigrator(); + $migrator->migrate($startSchema); + + try { + $migrator->migrate($endSchema); + $this->fail('Expected the conversion of an empty table to fail with ORA-22858'); + } catch (\Doctrine\DBAL\Exception\DriverException $e) { + $this->assertStringContainsString('ORA-22858', $e->getMessage()); + } + if ($this->connection->isTransactionActive()) { + $this->connection->rollBack(); + } + } + public function testAddingForeignKey(): void { $startSchema = new Schema([], [], $this->getSchemaConfig()); $table = $startSchema->createTable($this->tableName); From 8e4c48730e6b892386e0e8638cb99888bd4196ad Mon Sep 17 00:00:00 2001 From: James Manuel Date: Wed, 30 Sep 2026 12:57:15 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20=F0=9F=90=9B=20convert=20oc=5Fjobs.a?= =?UTF-8?q?rgument=20to=20CLOB=20out=20of=20place=20on=20Oracle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Oracle, preSchemaChange adds a CLOB copy of jobs.argument, copies and verifies the data, sets the copy NOT NULL if the original was, drops the original and renames the copy. changeSchema then sees TEXT and returns null. Each step is picked from the columns found, so an interrupted run resumes. Other platforms are unchanged. The new test runs the migration through MigrationService for each state oc_jobs can be in. Co-Authored-By: Claude Opus 5.5 Signed-off-by: James Manuel Assisted-by: ClaudeCode:claude-opus-5-5 --- .../Version34000Date20260318095645.php | 85 ++++++ .../Version34000Date20260318095645Test.php | 258 ++++++++++++++++++ 2 files changed, 343 insertions(+) create mode 100644 tests/Core/Migrations/Version34000Date20260318095645Test.php diff --git a/core/Migrations/Version34000Date20260318095645.php b/core/Migrations/Version34000Date20260318095645.php index 103cb68eed072..b13ad5ac04560 100644 --- a/core/Migrations/Version34000Date20260318095645.php +++ b/core/Migrations/Version34000Date20260318095645.php @@ -12,14 +12,80 @@ use Closure; use OCP\DB\ISchemaWrapper; use OCP\DB\Types; +use OCP\IDBConnection; use OCP\Migration\Attributes\ColumnType; use OCP\Migration\Attributes\ModifyColumn; use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; use Override; +use RuntimeException; #[ModifyColumn(table: 'jobs', name: 'argument', type: ColumnType::TEXT, description: 'Migrate background job arguments to a text column')] class Version34000Date20260318095645 extends SimpleMigrationStep { + private const COPY_COLUMN = 'argument_copy'; + + public function __construct( + private readonly IDBConnection $connection, + ) { + } + + /** + * Oracle cannot alter VARCHAR2 to CLOB in place (ORA-22858, even on an + * empty table), and Doctrine always restates the type in a MODIFY, so the + * conversion is done out of place here. changeSchema then sees TEXT and + * no-ops. Each step is driven by the columns found, so an interrupted run + * resumes where it stopped. + */ + #[Override] + public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + return; + } + + $tableName = $options['tablePrefix'] . 'jobs'; + $columns = $this->getOracleColumns($tableName); + $argument = $columns['argument'] ?? null; + $copy = $columns[self::COPY_COLUMN] ?? null; + + if ($copy === null && ($argument === null || $argument['type'] !== 'VARCHAR2')) { + return; + } + if ($copy !== null && ($copy['type'] !== 'CLOB' || ($argument !== null && $argument['type'] !== 'VARCHAR2'))) { + throw new RuntimeException("Unexpected state of $tableName: column " . self::COPY_COLUMN . ' exists alongside argument, but not as a CLOB copy of a VARCHAR2 argument. Resolve it manually, then rerun the upgrade.'); + } + + $output->info("Converting $tableName.argument to CLOB"); + $table = '"' . $tableName . '"'; + + if ($argument !== null) { + if ($copy === null) { + $this->connection->executeStatement('ALTER TABLE ' . $table . ' ADD ("argument_copy" CLOB)'); + $copy = ['type' => 'CLOB', 'nullable' => true]; + } + + $this->connection->executeStatement('UPDATE ' . $table . ' SET "argument_copy" = "argument"'); + $mismatches = (int)$this->connection->executeQuery( + 'SELECT COUNT(*) FROM ' . $table + . ' WHERE ("argument" IS NULL AND "argument_copy" IS NOT NULL)' + . ' OR ("argument" IS NOT NULL AND "argument_copy" IS NULL)' + . ' OR DBMS_LOB.COMPARE("argument_copy", TO_CLOB("argument")) <> 0' + )->fetchOne(); + if ($mismatches !== 0) { + throw new RuntimeException("$mismatches rows of $tableName were not copied to " . self::COPY_COLUMN . '. The original column is untouched; rerun the upgrade.'); + } + + // Tightening the copy before the drop makes a job written to the + // old column only fail instead of losing its argument. + if (!$argument['nullable'] && $copy['nullable']) { + $this->connection->executeStatement('ALTER TABLE ' . $table . ' MODIFY ("argument_copy" NOT NULL)'); + } + + $this->connection->executeStatement('ALTER TABLE ' . $table . ' DROP COLUMN "argument"'); + } + + $this->connection->executeStatement('ALTER TABLE ' . $table . ' RENAME COLUMN "argument_copy" TO "argument"'); + } + #[Override] public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ @@ -37,4 +103,23 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt return null; } + + /** + * @return array + */ + private function getOracleColumns(string $tableName): array { + $result = $this->connection->executeQuery( + 'SELECT column_name AS "name", data_type AS "type", nullable AS "nullable" FROM all_tab_columns' + . " WHERE owner = SYS_CONTEXT('USERENV', 'CURRENT_SCHEMA') AND table_name = ?", + [$tableName], + ); + + $columns = []; + foreach ($result->fetchAll() as $row) { + $columns[$row['name']] = ['type' => $row['type'], 'nullable' => $row['nullable'] === 'Y']; + } + $result->closeCursor(); + + return $columns; + } } diff --git a/tests/Core/Migrations/Version34000Date20260318095645Test.php b/tests/Core/Migrations/Version34000Date20260318095645Test.php new file mode 100644 index 0000000000000..ce98d1defc7bf --- /dev/null +++ b/tests/Core/Migrations/Version34000Date20260318095645Test.php @@ -0,0 +1,258 @@ +connection = Server::get(Connection::class); + $this->table = '"' . $this->connection->getPrefix() . 'jobs"'; + if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { + $this->installedNullable = $this->getOracleColumns()['argument']['nullable']; + } + } + + #[\Override] + protected function tearDown(): void { + if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { + $this->restoreInstalledState(); + } + $qb = $this->connection->getQueryBuilder(); + $qb->delete('jobs') + ->where($qb->expr()->eq('class', $qb->createNamedParameter(self::PROBE_CLASS))) + ->executeStatement(); + + parent::tearDown(); + } + + /** + * Fresh installs and app installs apply all steps as one schema diff, + * without preSchemaChange, so the installed test instance must end up + * with a text column and no leftover copy. + */ + public function testInstalledSchemaHasTextColumn(): void { + $table = (new SchemaWrapper($this->connection))->getTable('jobs'); + + $this->assertSame(Types::TEXT, $table->getColumn('argument')->getType()->getName()); + $this->assertFalse($table->hasColumn('argument_copy')); + } + + public function testMigrationKeepsConvertedColumn(): void { + $values = $this->insertProbeRows(); + + $this->executeMigration(); + + $table = (new SchemaWrapper($this->connection))->getTable('jobs'); + $this->assertSame(Types::TEXT, $table->getColumn('argument')->getType()->getName()); + $this->assertFalse($table->hasColumn('argument_copy')); + $this->assertSame($values, $this->getProbeArguments()); + } + + public static function oracleStateProvider(): array { + return [ + 'never upgraded' => ['setUpNeverUpgraded', false], + 'converted by hand' => ['setUpConvertedByHand', false], + 'converted by hand, nullable' => ['setUpConvertedByHandNullable', true], + 'interrupted after adding the copy' => ['setUpInterruptedAfterAdd', false], + 'interrupted before dropping the original' => ['setUpInterruptedBeforeDrop', false], + 'interrupted before renaming the copy' => ['setUpInterruptedBeforeRename', false], + ]; + } + + #[DataProvider('oracleStateProvider')] + public function testMigrationConvertsOracleColumn(string $setUpState, bool $expectedNullable): void { + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $this->markTestSkipped('The out-of-place conversion only runs on Oracle'); + } + + $values = $this->insertProbeRows(); + $this->$setUpState(); + + $this->executeMigration(); + + $columns = $this->getOracleColumns(); + $this->assertSame('CLOB', $columns['argument']['type']); + $this->assertSame($expectedNullable, $columns['argument']['nullable']); + $this->assertArrayNotHasKey('argument_copy', $columns); + $this->assertSame(Types::TEXT, (new SchemaWrapper($this->connection))->getTable('jobs') + ->getColumn('argument')->getType()->getName()); + $this->assertSame($values, $this->getProbeArguments()); + + $this->insertProbeRow('{"after":"migration"}'); + $this->assertSame([...$values, '{"after":"migration"}'], $this->getProbeArguments()); + } + + private function setUpNeverUpgraded(): void { + $tooLong = (int)$this->connection->executeQuery('SELECT COUNT(*) FROM ' . $this->table . ' WHERE DBMS_LOB.GETLENGTH("argument") > 4000')->fetchOne(); + if ($tooLong > 0) { + $this->markTestSkipped("$tooLong rows in oc_jobs do not fit the pre-migration VARCHAR2(4000) column"); + } + $this->replaceColumn('argument', 'VARCHAR2(4000)', 'DBMS_LOB.SUBSTR("argument", 4000, 1)'); + $this->alter('MODIFY ("argument" DEFAULT \'\' NOT NULL)'); + } + + private function setUpConvertedByHand(): void { + $this->setArgumentNullable(false); + } + + private function setUpConvertedByHandNullable(): void { + $this->setArgumentNullable(true); + } + + private function setUpInterruptedAfterAdd(): void { + $this->setUpNeverUpgraded(); + $this->alter('ADD ("argument_copy" CLOB)'); + } + + private function setUpInterruptedBeforeDrop(): void { + $this->setUpInterruptedAfterAdd(); + $this->connection->executeStatement('UPDATE ' . $this->table . ' SET "argument_copy" = "argument"'); + $this->alter('MODIFY ("argument_copy" NOT NULL)'); + } + + private function setUpInterruptedBeforeRename(): void { + $this->setUpInterruptedBeforeDrop(); + $this->alter('DROP COLUMN "argument"'); + } + + /** + * Puts oc_jobs.argument back to the installed CLOB, whatever state a + * failed test left it in, so the rest of the DB group is unaffected. + */ + private function restoreInstalledState(): void { + $columns = $this->getOracleColumns(); + if (isset($columns['argument_copy'])) { + if (isset($columns['argument'])) { + $this->alter('DROP COLUMN "argument_copy"'); + } else { + $this->alter('RENAME COLUMN "argument_copy" TO "argument"'); + } + $columns = $this->getOracleColumns(); + } + if ($columns['argument']['type'] !== 'CLOB') { + $this->replaceColumn('argument', 'CLOB', '"argument"'); + } + $this->setArgumentNullable($this->installedNullable); + } + + private function setArgumentNullable(bool $nullable): void { + if ($this->getOracleColumns()['argument']['nullable'] !== $nullable) { + $this->alter('MODIFY ("argument" ' . ($nullable ? 'NULL' : 'NOT NULL') . ')'); + } + } + + private function replaceColumn(string $column, string $type, string $copyExpression): void { + $this->alter('ADD ("replacement" ' . $type . ')'); + $this->connection->executeStatement('UPDATE ' . $this->table . ' SET "replacement" = ' . $copyExpression); + $this->alter('DROP COLUMN "' . $column . '"'); + $this->alter('RENAME COLUMN "replacement" TO "' . $column . '"'); + } + + private function alter(string $clause): void { + $this->connection->executeStatement('ALTER TABLE ' . $this->table . ' ' . $clause); + } + + private function executeMigration(): void { + (new MigrationService('core', $this->connection))->executeStep(self::VERSION); + + $qb = $this->connection->getQueryBuilder(); + $result = $qb->select('version') + ->from('migrations') + ->where($qb->expr()->eq('app', $qb->createNamedParameter('core'))) + ->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(self::VERSION))) + ->executeQuery(); + $this->assertSame([self::VERSION], $result->fetchFirstColumn()); + $result->closeCursor(); + } + + /** + * @return list + */ + private function insertProbeRows(): array { + $values = [ + '{"foo":"bar"}', + json_encode(['data' => str_repeat('x', 3900)]), + json_encode(['unicode' => 'üñïçødé 🥘 "quoted" \\']), + ]; + foreach ($values as $value) { + $this->insertProbeRow($value); + } + return $values; + } + + private function insertProbeRow(string $argument): void { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('jobs') + ->values([ + 'id' => $qb->createNamedParameter(Server::get(ISnowflakeGenerator::class)->nextId()), + 'class' => $qb->createNamedParameter(self::PROBE_CLASS), + 'argument' => $qb->createNamedParameter($argument), + 'argument_hash' => $qb->createNamedParameter(hash('sha256', $argument)), + ]) + ->executeStatement(); + } + + /** + * @return list + */ + private function getProbeArguments(): array { + $qb = $this->connection->getQueryBuilder(); + $result = $qb->select('argument') + ->from('jobs') + ->where($qb->expr()->eq('class', $qb->createNamedParameter(self::PROBE_CLASS))) + ->orderBy('id') + ->executeQuery(); + $arguments = $result->fetchFirstColumn(); + $result->closeCursor(); + return $arguments; + } + + /** + * @return array + */ + private function getOracleColumns(): array { + $rows = $this->connection->executeQuery( + 'SELECT column_name AS "name", data_type AS "type", nullable AS "nullable" FROM all_tab_columns' + . ' WHERE owner = SYS_CONTEXT(\'USERENV\', \'CURRENT_SCHEMA\') AND table_name = ?', + [$this->connection->getPrefix() . 'jobs'], + )->fetchAll(); + + $columns = []; + foreach ($rows as $row) { + $columns[$row['name']] = ['type' => $row['type'], 'nullable' => $row['nullable'] === 'Y']; + } + return $columns; + } +}