From d7da06847ca8d10133e965e6ceddfe60a39fd5c2 Mon Sep 17 00:00:00 2001 From: James Manuel Date: Wed, 8 Jul 2026 15:22:45 +0200 Subject: [PATCH 1/4] test(db): validate manual workaround for string to text conversion on Oracle Two Oracle-only tests (skipped elsewhere) executing against the CI Oracle 18 and 23 containers: - testStringToTextConversionWorkaroundOnOracle runs the manual add-copy-drop-rename SQL sequence verbatim, including the DBMS_LOB.COMPARE verification gate and the NOT NULL restore, asserts the data survives byte-for-byte (incl. multibyte), and asserts the resulting column introspects as text so the type guard of Version34000Date20260318095645 no-ops on a re-run of occ upgrade. - testChangeStringToTextEmptyTableFailsOnOracle documents that ORA-22858 fires independently of the column's contents, ruling out emptying the table as a workaround. Scratch branch for empirical validation only, not intended for merge in this form. Assisted-by: ClaudeCode:claude-fable-5 Co-Authored-By: Claude Fable 5 Signed-off-by: James Manuel --- tests/lib/DB/MigratorTest.php | 101 ++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index 099419884e072..7b5606e678dab 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -12,6 +12,7 @@ use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaConfig; +use Doctrine\DBAL\Types\Type; use OC\DB\Migrator; use OC\DB\OracleMigrator; use OC\DB\SQLiteMigrator; @@ -244,6 +245,106 @@ public function testColumnCommentsInUpdate(): void { $this->addToAssertionCount(1); } + /** + * Validates the manual workaround for the ORA-22858 upgrade failure: + * converting a string column to text via add-copy-drop-rename in plain + * SQL, after which the type guard of a migration like + * Version34000Date20260318095645 (oc_jobs.argument) sees a text column + * and no-ops. + */ + public function testStringToTextConversionWorkaroundOnOracle(): void { + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $this->markTestSkipped('Validates Oracle-specific workaround SQL'); + } + + $startSchema = new Schema([], [], $this->getSchemaConfig()); + $table = $startSchema->createTable($this->tableName); + $table->addColumn('id', Types::BIGINT); + $table->addColumn('argument', Types::STRING, [ + 'notnull' => true, + 'length' => 4000, + 'default' => '', + ]); + $table->addIndex(['id'], $this->tableName . '_id'); + + $migrator = $this->getMigrator(); + $migrator->migrate($startSchema); + + $values = [ + '{"foo":"bar"}', + json_encode(['data' => str_repeat('x', 3900)]), + json_encode(['emoji' => 'üñïçødé 🥘 "quoted" \\']), + ]; + foreach ($values as $i => $value) { + $this->connection->insert($this->tableName, ['id' => $i + 1, 'argument' => $value]); + } + + $quotedTable = $this->connection->quoteIdentifier($this->tableName); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' ADD ("argument2" CLOB)'); + $this->connection->executeStatement('UPDATE ' . $quotedTable . ' SET "argument2" = "argument"'); + + // verification gate: must be 0 before the old column may be dropped + $mismatches = $this->connection->executeQuery( + 'SELECT COUNT(*) FROM ' . $quotedTable + . ' WHERE ("argument" IS NOT NULL AND "argument2" IS NULL)' + . ' OR DBMS_LOB.COMPARE("argument2", TO_CLOB("argument")) <> 0' + )->fetchOne(); + $this->assertSame(0, (int)$mismatches); + + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' DROP COLUMN "argument"'); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' RENAME COLUMN "argument2" TO "argument"'); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' MODIFY ("argument" NOT NULL)'); + + $this->assertSame($values, $this->connection->executeQuery( + 'SELECT ' . $this->connection->quoteIdentifier('argument') + . ' FROM ' . $quotedTable + . ' ORDER BY ' . $this->connection->quoteIdentifier('id') . ' ASC' + )->fetchFirstColumn()); + + $columns = $this->connection->createSchemaManager()->listTableColumns($this->tableName); + $this->assertSame(Type::getType(Types::TEXT), $columns['argument']->getType()); + $this->assertTrue($columns['argument']->getNotnull()); + } + + /** + * 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 cb3586a89f79e1a518b00f411c6861652acbbc93 Mon Sep 17 00:00:00 2001 From: James Manuel Date: Wed, 8 Jul 2026 16:03:31 +0200 Subject: [PATCH 2/4] test(db): assert converted column type via SchemaWrapper On Oracle, Doctrine's listTableColumns() keys columns by their quoted name, so the plain 'argument' lookup returned null. Go through OC\DB\SchemaWrapper instead, which is also the interface the real migration type guard uses. Assisted-by: ClaudeCode:claude-fable-5 Co-Authored-By: Claude Fable 5 Signed-off-by: James Manuel --- tests/lib/DB/MigratorTest.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index 7b5606e678dab..aa505c0920c07 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -15,6 +15,7 @@ use Doctrine\DBAL\Types\Type; use OC\DB\Migrator; use OC\DB\OracleMigrator; +use OC\DB\SchemaWrapper; use OC\DB\SQLiteMigrator; use OCP\DB\Types; use OCP\EventDispatcher\IEventDispatcher; @@ -301,9 +302,12 @@ public function testStringToTextConversionWorkaroundOnOracle(): void { . ' ORDER BY ' . $this->connection->quoteIdentifier('id') . ' ASC' )->fetchFirstColumn()); - $columns = $this->connection->createSchemaManager()->listTableColumns($this->tableName); - $this->assertSame(Type::getType(Types::TEXT), $columns['argument']->getType()); - $this->assertTrue($columns['argument']->getNotnull()); + // same code path as the migration's type guard (ISchemaWrapper) + $schema = new SchemaWrapper($this->connection); + $column = $schema->getTable(substr($this->tableName, strlen($this->connection->getPrefix()))) + ->getColumn('argument'); + $this->assertSame(Type::getType(Types::TEXT), $column->getType()); + $this->assertTrue($column->getNotnull()); } /** From d54354d3365358b92a3fb89cce428a4a415a990d Mon Sep 17 00:00:00 2001 From: James Manuel Date: Fri, 10 Jul 2026 21:56:59 +0200 Subject: [PATCH 3/4] test(db): validate customer workaround script for ORA-22858 conversion Embeds the PL/SQL block of the customer-facing SQL*Plus script verbatim and validates it on the CI Oracle containers: - abort before any change when the backup is not confirmed - fresh conversion: gate-before-commit copy, column swap, NOT NULL restore, data intact byte-for-byte, type guard no-op via SchemaWrapper - idempotent re-run (already-converted state is a clean no-op) - resume of a run interrupted between DROP COLUMN and RENAME Assisted-by: ClaudeCode:claude-fable-5 Co-Authored-By: Claude Fable 5 Signed-off-by: James Manuel --- tests/lib/DB/MigratorTest.php | 256 ++++++++++++++++++++++++++++++++++ 1 file changed, 256 insertions(+) diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index aa505c0920c07..fa8a5b539b549 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -349,6 +349,262 @@ public function testChangeStringToTextEmptyTableFailsOnOracle(): void { } } + /** + * The PL/SQL block of the customer workaround script + * (nc-ora22858-workaround.sql) with the sqlplus substitution variables + * replaced. Must be kept identical to the block in the shipped script. + */ + private function getWorkaroundScriptBlock(string $tableName, string $backupConfirmed): string { + $block = <<<'SQL' +DECLARE + l_table CONSTANT VARCHAR2(128) := '&table_name'; + l_table_exists INTEGER; + l_old_type user_tab_columns.data_type%TYPE; + l_old_nullable user_tab_columns.nullable%TYPE; + l_new_type user_tab_columns.data_type%TYPE; + l_rows INTEGER; + l_nulls INTEGER; + l_count INTEGER; + + PROCEDURE fail(p_msg IN VARCHAR2) IS + BEGIN + RAISE_APPLICATION_ERROR(-20001, p_msg); + END; + + PROCEDURE say(p_msg IN VARCHAR2) IS + BEGIN + DBMS_OUTPUT.PUT_LINE(p_msg); + END; +BEGIN + IF LOWER(TRIM('&backup_confirmed')) NOT IN ('yes', 'y') THEN + fail('Aborted: restorable backup not confirmed. Nothing was changed.'); + END IF; + + SELECT COUNT(*) INTO l_table_exists + FROM user_tables WHERE table_name = l_table; + IF l_table_exists = 0 THEN + fail('Table "' || l_table || '" not found in this schema. Check the ' + || 'dbtableprefix in config.php and that you are connected as the ' + || 'Nextcloud database user. Nothing was changed.'); + END IF; + + BEGIN + SELECT data_type, nullable INTO l_old_type, l_old_nullable + FROM user_tab_columns + WHERE table_name = l_table AND column_name = 'argument'; + EXCEPTION WHEN NO_DATA_FOUND THEN + l_old_type := NULL; + END; + + BEGIN + SELECT data_type INTO l_new_type + FROM user_tab_columns + WHERE table_name = l_table AND column_name = 'argument2'; + EXCEPTION WHEN NO_DATA_FOUND THEN + l_new_type := NULL; + END; + + -- State: already converted (also the state after a successful run). + IF l_old_type = 'CLOB' AND l_new_type IS NULL THEN + say('Column "argument" is already CLOB - nothing to do.'); + say('Re-run `occ upgrade` if it has not completed yet.'); + RETURN; + END IF; + + -- State: unexpected - do not guess. + IF l_old_type = 'CLOB' AND l_new_type IS NOT NULL THEN + fail('Unexpected state: "argument" is already CLOB but "argument2" ' + || 'also exists. Manual inspection required. Nothing was changed.'); + END IF; + + -- State: interrupted between DROP and RENAME (copy was verified and + -- committed before the drop, so finishing the rename is safe). + IF l_old_type IS NULL AND l_new_type = 'CLOB' THEN + say('Resuming interrupted run: renaming "argument2" to "argument".'); + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" RENAME COLUMN "argument2" TO "argument"'; + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table + || '" WHERE "argument" IS NULL' INTO l_nulls; + IF l_nulls = 0 THEN + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" MODIFY ("argument" NOT NULL)'; + ELSE + say('WARNING: ' || l_nulls || ' NULL values present, NOT NULL ' + || 'constraint not restored - report this in the ticket.'); + END IF; + say('SUCCESS (resumed run completed). Re-run `occ upgrade` now.'); + RETURN; + END IF; + + IF l_old_type IS NULL THEN + fail('Column "argument" not found on "' || l_table || '" - ' + || 'unexpected schema. Nothing was changed.'); + END IF; + + IF l_old_type <> 'VARCHAR2' THEN + fail('Unexpected type for "argument": ' || l_old_type + || ' (expected VARCHAR2). Nothing was changed.'); + END IF; + + -- State: interrupted after ADD/copy but before DROP - the temporary + -- column may hold a partial copy; remove it and redo from scratch. + IF l_new_type IS NOT NULL THEN + say('Previous interrupted run detected: dropping "argument2" and ' + || 'redoing the copy.'); + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" DROP COLUMN "argument2"'; + END IF; + + -- Fresh conversion. + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table || '"' INTO l_rows; + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table + || '" WHERE "argument" IS NULL' INTO l_nulls; + say('Rows: ' || l_rows || ', NULL arguments: ' || l_nulls); + IF l_nulls > 0 THEN + fail(l_nulls || ' NULL values in "argument" - stop and report this ' + || 'in the ticket. Nothing was changed.'); + END IF; + + say('Adding temporary CLOB column and copying data...'); + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table || '" ADD ("argument2" CLOB)'; + EXECUTE IMMEDIATE 'UPDATE "' || l_table || '" SET "argument2" = "argument"'; + say('Copied ' || SQL%ROWCOUNT || ' rows.'); + + -- Verification gate - runs BEFORE the copy is committed. + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table + || '" WHERE ("argument" IS NOT NULL AND "argument2" IS NULL)' + || ' OR DBMS_LOB.COMPARE("argument2", TO_CLOB("argument")) <> 0' + INTO l_count; + IF l_count <> 0 THEN + ROLLBACK; + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" DROP COLUMN "argument2"'; + fail('Verification failed: ' || l_count || ' rows did not copy ' + || 'identically. The copy was rolled back and the temporary ' + || 'column removed - the original column is untouched. Report ' + || 'this in the ticket.'); + END IF; + COMMIT; + say('Copy verified: 0 mismatches. Swapping columns...'); + + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table || '" DROP COLUMN "argument"'; + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" RENAME COLUMN "argument2" TO "argument"'; + IF l_old_nullable = 'N' THEN + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" MODIFY ("argument" NOT NULL)'; + END IF; + + -- Final checks. + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table || '"' INTO l_count; + IF l_count <> l_rows THEN + fail('Row count changed during conversion: ' || l_rows || ' -> ' + || l_count || '. Restore from backup and report this.'); + END IF; + SELECT data_type INTO l_old_type + FROM user_tab_columns + WHERE table_name = l_table AND column_name = 'argument'; + IF l_old_type <> 'CLOB' THEN + fail('Post-check failed: "argument" is ' || l_old_type + || ', expected CLOB.'); + END IF; + + say('SUCCESS: "argument" converted to CLOB, ' || l_rows + || ' rows verified intact. Re-run `occ upgrade` now.'); +END; +SQL; + return str_replace( + ['&table_name', '&backup_confirmed'], + [$tableName, $backupConfirmed], + $block, + ); + } + + private function createWorkaroundTestTable(): array { + $startSchema = new Schema([], [], $this->getSchemaConfig()); + $table = $startSchema->createTable($this->tableName); + $table->addColumn('id', Types::BIGINT); + $table->addColumn('argument', Types::STRING, [ + 'notnull' => true, + 'length' => 4000, + 'default' => '', + ]); + $table->addIndex(['id'], $this->tableName . '_id'); + $this->getMigrator()->migrate($startSchema); + + $values = [ + '{"foo":"bar"}', + json_encode(['data' => str_repeat('x', 3900)]), + json_encode(['emoji' => 'üñïçødé 🥘 "quoted" \\']), + ]; + foreach ($values as $i => $value) { + $this->connection->insert($this->tableName, ['id' => $i + 1, 'argument' => $value]); + } + return $values; + } + + private function assertWorkaroundResult(array $values): void { + $this->assertSame($values, $this->connection->executeQuery( + 'SELECT ' . $this->connection->quoteIdentifier('argument') + . ' FROM ' . $this->connection->quoteIdentifier($this->tableName) + . ' ORDER BY ' . $this->connection->quoteIdentifier('id') . ' ASC' + )->fetchFirstColumn()); + + $schema = new SchemaWrapper($this->connection); + $column = $schema->getTable(substr($this->tableName, strlen($this->connection->getPrefix()))) + ->getColumn('argument'); + $this->assertSame(Type::getType(Types::TEXT), $column->getType()); + $this->assertTrue($column->getNotnull()); + } + + /** + * Validates the customer workaround script for the ORA-22858 upgrade + * failure: abort without backup confirmation, fresh conversion, and + * idempotent re-run. + */ + public function testWorkaroundScriptOnOracle(): void { + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $this->markTestSkipped('Validates the Oracle-specific workaround script'); + } + + $values = $this->createWorkaroundTestTable(); + + try { + $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'no')); + $this->fail('Expected the script to abort without backup confirmation'); + } catch (\Doctrine\DBAL\Exception\DriverException $e) { + $this->assertStringContainsString('backup not confirmed', $e->getMessage()); + } + + $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'yes')); + $this->assertWorkaroundResult($values); + + // re-running must be a clean no-op + $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'yes')); + $this->assertWorkaroundResult($values); + } + + /** + * The script must finish a run that was interrupted between dropping the + * original column and renaming the copy. + */ + public function testWorkaroundScriptResumesInterruptedRunOnOracle(): void { + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $this->markTestSkipped('Validates the Oracle-specific workaround script'); + } + + $values = $this->createWorkaroundTestTable(); + + // reproduce the state after DROP COLUMN but before RENAME + $quotedTable = $this->connection->quoteIdentifier($this->tableName); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' ADD ("argument2" CLOB)'); + $this->connection->executeStatement('UPDATE ' . $quotedTable . ' SET "argument2" = "argument"'); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' DROP COLUMN "argument"'); + + $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'yes')); + $this->assertWorkaroundResult($values); + } + public function testAddingForeignKey(): void { $startSchema = new Schema([], [], $this->getSchemaConfig()); $table = $startSchema->createTable($this->tableName); From 3532beaba87a3b6e61df9cbb156714054e5940fc Mon Sep 17 00:00:00 2001 From: James Manuel Date: Fri, 10 Jul 2026 22:28:04 +0200 Subject: [PATCH 4/4] test(db): harden ORA-22858 workaround script per adversarial review Fixes from a cold review of the customer script: - empty input at the backup prompt no longer bypasses the abort (TRIM('') is NULL, NULL NOT IN (...) is UNKNOWN): ACCEPT gets a DEFAULT 'no' and the check wraps the value in NVL - a run interrupted between RENAME and the NOT NULL restore is now repaired on re-run instead of reported as done (restore_not_null runs in the already-converted branch) - the redo branch only drops an argument2 column of type CLOB; any other type aborts as an unexpected state - the copy column is made NOT NULL before the original is dropped, so a straggling writer fails loudly instead of losing a row's argument - verification gate covers the argument-NULL/argument2-NOT-NULL asymmetry; DEFAULT '' is restored on the rebuilt column The script now lives in tests/data/ and the test reads the PL/SQL block from it, so CI validates the literal shipped artifact instead of a manually-synced copy. New coverage: empty-input abort, foreign argument2 refusal, NOT NULL repair on re-run. Assisted-by: ClaudeCode:claude-fable-5 Co-Authored-By: Claude Fable 5 Signed-off-by: James Manuel --- tests/data/nc-ora22858-workaround.sql | 255 ++++++++++++++++++++++++++ tests/lib/DB/MigratorTest.php | 212 +++++---------------- 2 files changed, 302 insertions(+), 165 deletions(-) create mode 100644 tests/data/nc-ora22858-workaround.sql diff --git a/tests/data/nc-ora22858-workaround.sql b/tests/data/nc-ora22858-workaround.sql new file mode 100644 index 0000000000000..433301ab45cd9 --- /dev/null +++ b/tests/data/nc-ora22858-workaround.sql @@ -0,0 +1,255 @@ +-- Nextcloud ORA-22858 upgrade failure: manual conversion of the jobs +-- "argument" column from VARCHAR2(4000) to CLOB. +-- +-- Context: `occ upgrade` to Nextcloud >= 32.0.7 / 33.0.1 / 34.x fails on +-- Oracle with "ORA-22858: invalid alteration of datatype" in core migration +-- 34000Date20260318095645, because Oracle cannot ALTER a VARCHAR2 column to +-- CLOB in place. This script performs the conversion the way Oracle +-- requires (add CLOB column, copy, verify, drop, rename). Afterwards, +-- re-run `occ upgrade`: the migration detects the already-converted column +-- and completes without further changes. +-- +-- Run as the Nextcloud database user, with Nextcloud in maintenance mode. +-- IMPORTANT: also verify that cron jobs and web/app workers are actually +-- stopped, not merely that maintenance mode is set. +-- sqlplus @ @nc-ora22858-workaround.sql +-- (SQLcl works identically. If you use a different client: replace the two +-- substitution variables - the table name and the backup confirmation - in +-- the DECLARE...END; block below with literal values and execute that block +-- on its own. The prompts, SPOOL and the state listings are the only +-- SQL*Plus-specific parts.) +-- +-- Safety properties: +-- * Aborts before any change unless a restorable backup is explicitly +-- confirmed (empty input aborts). +-- * Inspects the current column state first and handles a previously +-- interrupted run: it is safe to run this script multiple times. +-- * The data copy is verified (DBMS_LOB.COMPARE) BEFORE it is committed; +-- on any mismatch everything is rolled back and the temporary column is +-- removed - the schema is left exactly as it was found. +-- * The copy column is locked down (NOT NULL) before the original column +-- is dropped, so a forgotten writer fails loudly instead of losing data. +-- * A full transcript is written to nc_ora22858_workaround.log - attach +-- it to the support ticket. +-- +-- Runtime scales with table size (the verification compares every row); +-- on a normally-sized jobs table this completes in seconds. + +SET SERVEROUTPUT ON SIZE UNLIMITED +SET VERIFY OFF +SET FEEDBACK ON +WHENEVER SQLERROR EXIT FAILURE ROLLBACK + +SPOOL nc_ora22858_workaround.log + +ACCEPT table_name CHAR DEFAULT 'oc_jobs' PROMPT 'Jobs table name (check dbtableprefix in config.php) [oc_jobs]: ' +ACCEPT backup_confirmed CHAR DEFAULT 'no' PROMPT 'Is a restorable database backup confirmed? (yes/no) [no]: ' + +PROMPT +PROMPT === Column state before === +SELECT column_name, data_type, data_length, nullable + FROM user_tab_columns + WHERE table_name = '&table_name' + ORDER BY column_id; + +DECLARE + l_table CONSTANT VARCHAR2(128) := '&table_name'; + l_table_exists INTEGER; + l_old_type user_tab_columns.data_type%TYPE; + l_old_nullable user_tab_columns.nullable%TYPE; + l_new_type user_tab_columns.data_type%TYPE; + l_rows INTEGER; + l_nulls INTEGER; + l_count INTEGER; + + PROCEDURE fail(p_msg IN VARCHAR2) IS + BEGIN + RAISE_APPLICATION_ERROR(-20001, p_msg); + END; + + PROCEDURE say(p_msg IN VARCHAR2) IS + BEGIN + DBMS_OUTPUT.PUT_LINE(p_msg); + END; + + -- Restore NOT NULL on "argument" if it is currently nullable and holds + -- no NULLs. Used by every path that can encounter a rebuilt column, so + -- a run interrupted before the constraint was restored is repaired by + -- the next run. + PROCEDURE restore_not_null IS + l_nullable user_tab_columns.nullable%TYPE; + l_null_rows INTEGER; + BEGIN + SELECT nullable INTO l_nullable + FROM user_tab_columns + WHERE table_name = l_table AND column_name = 'argument'; + IF l_nullable = 'Y' THEN + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table + || '" WHERE "argument" IS NULL' INTO l_null_rows; + IF l_null_rows = 0 THEN + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" MODIFY ("argument" NOT NULL)'; + say('Restored NOT NULL constraint on "argument".'); + ELSE + say('WARNING: ' || l_null_rows || ' NULL values present, NOT ' + || 'NULL constraint not restored - report this in the ticket.'); + END IF; + END IF; + END; +BEGIN + IF NVL(LOWER(TRIM('&backup_confirmed')), 'no') NOT IN ('yes', 'y') THEN + fail('Aborted: restorable backup not confirmed. Nothing was changed.'); + END IF; + + SELECT COUNT(*) INTO l_table_exists + FROM user_tables WHERE table_name = l_table; + IF l_table_exists = 0 THEN + fail('Table "' || l_table || '" not found in this schema. Check the ' + || 'dbtableprefix in config.php and that you are connected as the ' + || 'Nextcloud database user. Nothing was changed.'); + END IF; + + BEGIN + SELECT data_type, nullable INTO l_old_type, l_old_nullable + FROM user_tab_columns + WHERE table_name = l_table AND column_name = 'argument'; + EXCEPTION WHEN NO_DATA_FOUND THEN + l_old_type := NULL; + END; + + BEGIN + SELECT data_type INTO l_new_type + FROM user_tab_columns + WHERE table_name = l_table AND column_name = 'argument2'; + EXCEPTION WHEN NO_DATA_FOUND THEN + l_new_type := NULL; + END; + + -- State: already converted (also the state after a successful run, or + -- after a run interrupted before the NOT NULL constraint was restored). + IF l_old_type = 'CLOB' AND l_new_type IS NULL THEN + restore_not_null; + say('Column "argument" is already CLOB - nothing else to do.'); + say('Re-run `occ upgrade` if it has not completed yet.'); + RETURN; + END IF; + + -- State: unexpected - do not guess. + IF l_old_type = 'CLOB' AND l_new_type IS NOT NULL THEN + fail('Unexpected state: "argument" is already CLOB but "argument2" ' + || 'also exists. Manual inspection required. Nothing was changed.'); + END IF; + + -- State: interrupted between DROP and RENAME (copy was verified and + -- committed before the drop, so finishing the rename is safe). + IF l_old_type IS NULL AND l_new_type = 'CLOB' THEN + say('Resuming interrupted run: renaming "argument2" to "argument".'); + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" RENAME COLUMN "argument2" TO "argument"'; + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" MODIFY ("argument" DEFAULT '''')'; + restore_not_null; + say('SUCCESS (resumed run completed). Re-run `occ upgrade` now.'); + RETURN; + END IF; + + IF l_old_type IS NULL THEN + fail('Column "argument" not found on "' || l_table || '" - ' + || 'unexpected schema. Nothing was changed.'); + END IF; + + IF l_old_type <> 'VARCHAR2' THEN + fail('Unexpected type for "argument": ' || l_old_type + || ' (expected VARCHAR2). Nothing was changed.'); + END IF; + + -- State: interrupted after ADD/copy but before DROP - the temporary + -- CLOB column may hold a partial copy; remove it and redo from scratch. + -- An "argument2" of any other type was not created by this script. + IF l_new_type IS NOT NULL AND l_new_type <> 'CLOB' THEN + fail('Unexpected state: column "argument2" exists with type ' + || l_new_type || ' - not created by this script. Manual ' + || 'inspection required. Nothing was changed.'); + ELSIF l_new_type IS NOT NULL THEN + say('Previous interrupted run detected: dropping "argument2" and ' + || 'redoing the copy.'); + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" DROP COLUMN "argument2"'; + END IF; + + -- Fresh conversion. + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table || '"' INTO l_rows; + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table + || '" WHERE "argument" IS NULL' INTO l_nulls; + say('Rows: ' || l_rows || ', NULL arguments: ' || l_nulls); + IF l_nulls > 0 THEN + fail(l_nulls || ' NULL values in "argument" - stop and report this ' + || 'in the ticket. Nothing was changed.'); + END IF; + + say('Adding temporary CLOB column and copying data...'); + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table || '" ADD ("argument2" CLOB)'; + EXECUTE IMMEDIATE 'UPDATE "' || l_table || '" SET "argument2" = "argument"'; + say('Copied ' || SQL%ROWCOUNT || ' rows.'); + + -- Verification gate - runs BEFORE the copy is committed. + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table + || '" WHERE ("argument" IS NOT NULL AND "argument2" IS NULL)' + || ' OR ("argument" IS NULL AND "argument2" IS NOT NULL)' + || ' OR DBMS_LOB.COMPARE("argument2", TO_CLOB("argument")) <> 0' + INTO l_count; + IF l_count <> 0 THEN + ROLLBACK; + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" DROP COLUMN "argument2"'; + fail('Verification failed: ' || l_count || ' rows did not copy ' + || 'identically. The copy was rolled back and the temporary ' + || 'column removed - the original column is untouched. Report ' + || 'this in the ticket.'); + END IF; + COMMIT; + say('Copy verified: 0 mismatches.'); + + -- Lock the copy down before dropping the original: a straggling writer + -- (cron/web worker that should have been stopped) now fails loudly on + -- insert instead of silently losing its "argument" value in the drop. + IF l_old_nullable = 'N' THEN + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" MODIFY ("argument2" NOT NULL)'; + END IF; + + say('Swapping columns...'); + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table || '" DROP COLUMN "argument"'; + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" RENAME COLUMN "argument2" TO "argument"'; + EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table + || '" MODIFY ("argument" DEFAULT '''')'; + + -- Final checks. + EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table || '"' INTO l_count; + IF l_count <> l_rows THEN + fail('Row count changed during conversion: ' || l_rows || ' -> ' + || l_count || '. Restore from backup and report this.'); + END IF; + SELECT data_type INTO l_old_type + FROM user_tab_columns + WHERE table_name = l_table AND column_name = 'argument'; + IF l_old_type <> 'CLOB' THEN + fail('Post-check failed: "argument" is ' || l_old_type + || ', expected CLOB.'); + END IF; + + say('SUCCESS: "argument" converted to CLOB, ' || l_rows + || ' rows verified intact. Re-run `occ upgrade` now.'); +END; +/ + +PROMPT +PROMPT === Column state after === +SELECT column_name, data_type, data_length, nullable + FROM user_tab_columns + WHERE table_name = '&table_name' + ORDER BY column_id; + +SPOOL OFF +EXIT diff --git a/tests/lib/DB/MigratorTest.php b/tests/lib/DB/MigratorTest.php index fa8a5b539b549..b74d03381f027 100644 --- a/tests/lib/DB/MigratorTest.php +++ b/tests/lib/DB/MigratorTest.php @@ -350,173 +350,18 @@ public function testChangeStringToTextEmptyTableFailsOnOracle(): void { } /** - * The PL/SQL block of the customer workaround script - * (nc-ora22858-workaround.sql) with the sqlplus substitution variables - * replaced. Must be kept identical to the block in the shipped script. + * The PL/SQL block of the customer workaround script, read from the + * shipped script file so CI validates the literal artifact, with the + * sqlplus substitution variables replaced the way sqlplus would. */ private function getWorkaroundScriptBlock(string $tableName, string $backupConfirmed): string { - $block = <<<'SQL' -DECLARE - l_table CONSTANT VARCHAR2(128) := '&table_name'; - l_table_exists INTEGER; - l_old_type user_tab_columns.data_type%TYPE; - l_old_nullable user_tab_columns.nullable%TYPE; - l_new_type user_tab_columns.data_type%TYPE; - l_rows INTEGER; - l_nulls INTEGER; - l_count INTEGER; - - PROCEDURE fail(p_msg IN VARCHAR2) IS - BEGIN - RAISE_APPLICATION_ERROR(-20001, p_msg); - END; - - PROCEDURE say(p_msg IN VARCHAR2) IS - BEGIN - DBMS_OUTPUT.PUT_LINE(p_msg); - END; -BEGIN - IF LOWER(TRIM('&backup_confirmed')) NOT IN ('yes', 'y') THEN - fail('Aborted: restorable backup not confirmed. Nothing was changed.'); - END IF; - - SELECT COUNT(*) INTO l_table_exists - FROM user_tables WHERE table_name = l_table; - IF l_table_exists = 0 THEN - fail('Table "' || l_table || '" not found in this schema. Check the ' - || 'dbtableprefix in config.php and that you are connected as the ' - || 'Nextcloud database user. Nothing was changed.'); - END IF; - - BEGIN - SELECT data_type, nullable INTO l_old_type, l_old_nullable - FROM user_tab_columns - WHERE table_name = l_table AND column_name = 'argument'; - EXCEPTION WHEN NO_DATA_FOUND THEN - l_old_type := NULL; - END; - - BEGIN - SELECT data_type INTO l_new_type - FROM user_tab_columns - WHERE table_name = l_table AND column_name = 'argument2'; - EXCEPTION WHEN NO_DATA_FOUND THEN - l_new_type := NULL; - END; - - -- State: already converted (also the state after a successful run). - IF l_old_type = 'CLOB' AND l_new_type IS NULL THEN - say('Column "argument" is already CLOB - nothing to do.'); - say('Re-run `occ upgrade` if it has not completed yet.'); - RETURN; - END IF; - - -- State: unexpected - do not guess. - IF l_old_type = 'CLOB' AND l_new_type IS NOT NULL THEN - fail('Unexpected state: "argument" is already CLOB but "argument2" ' - || 'also exists. Manual inspection required. Nothing was changed.'); - END IF; - - -- State: interrupted between DROP and RENAME (copy was verified and - -- committed before the drop, so finishing the rename is safe). - IF l_old_type IS NULL AND l_new_type = 'CLOB' THEN - say('Resuming interrupted run: renaming "argument2" to "argument".'); - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table - || '" RENAME COLUMN "argument2" TO "argument"'; - EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table - || '" WHERE "argument" IS NULL' INTO l_nulls; - IF l_nulls = 0 THEN - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table - || '" MODIFY ("argument" NOT NULL)'; - ELSE - say('WARNING: ' || l_nulls || ' NULL values present, NOT NULL ' - || 'constraint not restored - report this in the ticket.'); - END IF; - say('SUCCESS (resumed run completed). Re-run `occ upgrade` now.'); - RETURN; - END IF; - - IF l_old_type IS NULL THEN - fail('Column "argument" not found on "' || l_table || '" - ' - || 'unexpected schema. Nothing was changed.'); - END IF; - - IF l_old_type <> 'VARCHAR2' THEN - fail('Unexpected type for "argument": ' || l_old_type - || ' (expected VARCHAR2). Nothing was changed.'); - END IF; - - -- State: interrupted after ADD/copy but before DROP - the temporary - -- column may hold a partial copy; remove it and redo from scratch. - IF l_new_type IS NOT NULL THEN - say('Previous interrupted run detected: dropping "argument2" and ' - || 'redoing the copy.'); - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table - || '" DROP COLUMN "argument2"'; - END IF; - - -- Fresh conversion. - EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table || '"' INTO l_rows; - EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table - || '" WHERE "argument" IS NULL' INTO l_nulls; - say('Rows: ' || l_rows || ', NULL arguments: ' || l_nulls); - IF l_nulls > 0 THEN - fail(l_nulls || ' NULL values in "argument" - stop and report this ' - || 'in the ticket. Nothing was changed.'); - END IF; - - say('Adding temporary CLOB column and copying data...'); - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table || '" ADD ("argument2" CLOB)'; - EXECUTE IMMEDIATE 'UPDATE "' || l_table || '" SET "argument2" = "argument"'; - say('Copied ' || SQL%ROWCOUNT || ' rows.'); - - -- Verification gate - runs BEFORE the copy is committed. - EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table - || '" WHERE ("argument" IS NOT NULL AND "argument2" IS NULL)' - || ' OR DBMS_LOB.COMPARE("argument2", TO_CLOB("argument")) <> 0' - INTO l_count; - IF l_count <> 0 THEN - ROLLBACK; - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table - || '" DROP COLUMN "argument2"'; - fail('Verification failed: ' || l_count || ' rows did not copy ' - || 'identically. The copy was rolled back and the temporary ' - || 'column removed - the original column is untouched. Report ' - || 'this in the ticket.'); - END IF; - COMMIT; - say('Copy verified: 0 mismatches. Swapping columns...'); - - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table || '" DROP COLUMN "argument"'; - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table - || '" RENAME COLUMN "argument2" TO "argument"'; - IF l_old_nullable = 'N' THEN - EXECUTE IMMEDIATE 'ALTER TABLE "' || l_table - || '" MODIFY ("argument" NOT NULL)'; - END IF; - - -- Final checks. - EXECUTE IMMEDIATE 'SELECT COUNT(*) FROM "' || l_table || '"' INTO l_count; - IF l_count <> l_rows THEN - fail('Row count changed during conversion: ' || l_rows || ' -> ' - || l_count || '. Restore from backup and report this.'); - END IF; - SELECT data_type INTO l_old_type - FROM user_tab_columns - WHERE table_name = l_table AND column_name = 'argument'; - IF l_old_type <> 'CLOB' THEN - fail('Post-check failed: "argument" is ' || l_old_type - || ', expected CLOB.'); - END IF; - - say('SUCCESS: "argument" converted to CLOB, ' || l_rows - || ' rows verified intact. Re-run `occ upgrade` now.'); -END; -SQL; + $script = file_get_contents(\OC::$SERVERROOT . '/tests/data/nc-ora22858-workaround.sql'); + $this->assertNotFalse($script, 'workaround script file not readable'); + $this->assertSame(1, preg_match('/^DECLARE\R.*?^END;/ms', $script, $matches), 'PL/SQL block not found in workaround script'); return str_replace( ['&table_name', '&backup_confirmed'], [$tableName, $backupConfirmed], - $block, + $matches[0], ); } @@ -569,12 +414,26 @@ public function testWorkaroundScriptOnOracle(): void { $values = $this->createWorkaroundTestTable(); + // pressing enter at the prompt yields an empty value - must abort + foreach (['no', ''] as $notConfirmed) { + try { + $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, $notConfirmed)); + $this->fail('Expected the script to abort without backup confirmation'); + } catch (\Doctrine\DBAL\Exception\DriverException $e) { + $this->assertStringContainsString('backup not confirmed', $e->getMessage()); + } + } + + // an argument2 column this script did not create must not be dropped + $quotedTable = $this->connection->quoteIdentifier($this->tableName); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' ADD ("argument2" NUMBER)'); try { - $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'no')); - $this->fail('Expected the script to abort without backup confirmation'); + $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'yes')); + $this->fail('Expected the script to refuse a foreign argument2 column'); } catch (\Doctrine\DBAL\Exception\DriverException $e) { - $this->assertStringContainsString('backup not confirmed', $e->getMessage()); + $this->assertStringContainsString('argument2', $e->getMessage()); } + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' DROP COLUMN "argument2"'); $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'yes')); $this->assertWorkaroundResult($values); @@ -605,6 +464,29 @@ public function testWorkaroundScriptResumesInterruptedRunOnOracle(): void { $this->assertWorkaroundResult($values); } + /** + * A run interrupted between the RENAME and the NOT NULL restore leaves + * a nullable CLOB column - a re-run must repair the constraint instead + * of declaring the state done. + */ + public function testWorkaroundScriptRestoresNotNullOnOracle(): void { + if ($this->connection->getDatabaseProvider() !== IDBConnection::PLATFORM_ORACLE) { + $this->markTestSkipped('Validates the Oracle-specific workaround script'); + } + + $values = $this->createWorkaroundTestTable(); + + // reproduce the state after RENAME but before the NOT NULL restore + $quotedTable = $this->connection->quoteIdentifier($this->tableName); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' ADD ("argument2" CLOB)'); + $this->connection->executeStatement('UPDATE ' . $quotedTable . ' SET "argument2" = "argument"'); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' DROP COLUMN "argument"'); + $this->connection->executeStatement('ALTER TABLE ' . $quotedTable . ' RENAME COLUMN "argument2" TO "argument"'); + + $this->connection->executeStatement($this->getWorkaroundScriptBlock($this->tableName, 'yes')); + $this->assertWorkaroundResult($values); + } + public function testAddingForeignKey(): void { $startSchema = new Schema([], [], $this->getSchemaConfig()); $table = $startSchema->createTable($this->tableName);