Skip to content

Commit 67eef32

Browse files
susnuxsusnux
committed
fix(DB): support up to 63 character long table and index names
We do not support Oracle 11 anymore but at least Oracle 12c (12.2). So the limitation is gone (Oracle now supports up to 128 character long names). Instead we are now limited by MySQL (64 characters) and PostgreSQL (63 characters). Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent eb9a3d3 commit 67eef32

4 files changed

Lines changed: 707 additions & 473 deletions

File tree

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3515,14 +3515,6 @@
35153515
<code><![CDATA[0]]></code>
35163516
</TypeDoesNotContainType>
35173517
</file>
3518-
<file src="lib/private/DB/MigrationService.php">
3519-
<LessSpecificReturnStatement>
3520-
<code><![CDATA[$s]]></code>
3521-
</LessSpecificReturnStatement>
3522-
<MoreSpecificReturnType>
3523-
<code><![CDATA[IMigrationStep]]></code>
3524-
</MoreSpecificReturnType>
3525-
</file>
35263518
<file src="lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php">
35273519
<ImplicitToStringCast>
35283520
<code><![CDATA[$this->functionBuilder->lower($x)]]></code>

‎lib/private/DB/MigrationService.php‎

Lines changed: 139 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@
1717
use OC\Migration\SimpleOutput;
1818
use OCP\App\IAppManager;
1919
use OCP\AppFramework\App;
20-
use OCP\AppFramework\QueryException;
2120
use OCP\DB\ISchemaWrapper;
2221
use OCP\DB\Types;
22+
use OCP\IConfig;
2323
use OCP\IDBConnection;
2424
use OCP\Migration\IMigrationStep;
2525
use OCP\Migration\IOutput;
2626
use OCP\Server;
27+
use Psr\Container\NotFoundExceptionInterface;
2728
use Psr\Log\LoggerInterface;
2829

2930
class MigrationService {
@@ -47,6 +48,7 @@ public function __construct(
4748
?LoggerInterface $logger = null,
4849
) {
4950
$this->appName = $appName;
51+
$this->checkOracle = false;
5052
$this->connection = $connection;
5153
if ($logger === null) {
5254
$this->logger = Server::get(LoggerInterface::class);
@@ -103,7 +105,7 @@ private function createMigrationTable(): bool {
103105
return false;
104106
}
105107

106-
if ($this->connection->tableExists('migrations') && \OC::$server->getConfig()->getAppValue('core', 'vendor', '') !== 'owncloud') {
108+
if ($this->connection->tableExists('migrations') && \OCP\Server::get(IConfig::class)->getAppValue('core', 'vendor', '') !== 'owncloud') {
107109
$this->migrationTableCreated = true;
108110
return false;
109111
}
@@ -282,7 +284,7 @@ private function shallBeExecuted($m, $knownMigrations) {
282284
/**
283285
* @param string $version
284286
*/
285-
private function markAsExecuted($version) {
287+
private function markAsExecuted($version): void {
286288
$this->connection->insertIfNotExist('*PREFIX*migrations', [
287289
'app' => $this->appName,
288290
'version' => $version
@@ -343,7 +345,7 @@ private function getRelativeVersion(string $version, int $delta): ?string {
343345

344346
$versions = $this->getAvailableVersions();
345347
array_unshift($versions, '0');
346-
/** @var int $offset */
348+
/** @var int|false $offset */
347349
$offset = array_search($version, $versions, true);
348350
if ($offset === false || !isset($versions[$offset + $delta])) {
349351
// Unknown version or delta out of bounds.
@@ -358,8 +360,7 @@ private function getCurrentVersion(): string {
358360
if (count($m) === 0) {
359361
return '0';
360362
}
361-
$migrations = array_values($m);
362-
return @end($migrations);
363+
return @end($m);
363364
}
364365

365366
/**
@@ -431,10 +432,11 @@ public function migrateSchemaOnly(string $to = 'latest'): void {
431432
if ($toSchema instanceof SchemaWrapper) {
432433
$this->output->debug('- Checking target database schema');
433434
$targetSchema = $toSchema->getWrappedSchema();
435+
$beforeSchema = $this->connection->createSchema();
434436
$this->ensureUniqueNamesConstraints($targetSchema, true);
437+
$this->ensureNamingConstraints($beforeSchema, $targetSchema, \strlen($this->connection->getPrefix()));
435438
if ($this->checkOracle) {
436-
$beforeSchema = $this->connection->createSchema();
437-
$this->ensureOracleConstraints($beforeSchema, $targetSchema, strlen($this->connection->getPrefix()));
439+
$this->ensureOracleConstraints($beforeSchema, $targetSchema);
438440
}
439441

440442
$this->output->debug('- Migrate database schema');
@@ -472,21 +474,21 @@ public function describeMigrationStep($to = 'latest') {
472474
* @throws \InvalidArgumentException
473475
*/
474476
public function createInstance($version) {
477+
/** @psalm-var class-string<IMigrationStep> $class */
475478
$class = $this->getClass($version);
476479
try {
477480
$s = \OCP\Server::get($class);
478-
479-
if (!$s instanceof IMigrationStep) {
480-
throw new \InvalidArgumentException('Not a valid migration');
481-
}
482-
} catch (QueryException $e) {
481+
} catch (NotFoundExceptionInterface) {
483482
if (class_exists($class)) {
484483
$s = new $class();
485484
} else {
486485
throw new \InvalidArgumentException("Migration step '$class' is unknown");
487486
}
488487
}
489488

489+
if (!$s instanceof IMigrationStep) {
490+
throw new \InvalidArgumentException('Not a valid migration');
491+
}
490492
return $s;
491493
}
492494

@@ -497,7 +499,7 @@ public function createInstance($version) {
497499
* @param bool $schemaOnly
498500
* @throws \InvalidArgumentException
499501
*/
500-
public function executeStep($version, $schemaOnly = false) {
502+
public function executeStep($version, $schemaOnly = false): void {
501503
$instance = $this->createInstance($version);
502504

503505
if (!$schemaOnly) {
@@ -512,10 +514,11 @@ public function executeStep($version, $schemaOnly = false) {
512514

513515
if ($toSchema instanceof SchemaWrapper) {
514516
$targetSchema = $toSchema->getWrappedSchema();
517+
$sourceSchema = $this->connection->createSchema();
515518
$this->ensureUniqueNamesConstraints($targetSchema, $schemaOnly);
519+
$this->ensureNamingConstraints($sourceSchema, $targetSchema, \strlen($this->connection->getPrefix()));
516520
if ($this->checkOracle) {
517-
$sourceSchema = $this->connection->createSchema();
518-
$this->ensureOracleConstraints($sourceSchema, $targetSchema, strlen($this->connection->getPrefix()));
521+
$this->ensureOracleConstraints($sourceSchema, $targetSchema);
519522
}
520523
$this->connection->migrateToSchema($targetSchema);
521524
$toSchema->performDropTableCalls();
@@ -531,12 +534,108 @@ public function executeStep($version, $schemaOnly = false) {
531534
}
532535

533536
/**
537+
* Enforces some naming conventions to make sure tables can be used on all supported database engines.
538+
*
534539
* Naming constraints:
535-
* - Tables names must be 30 chars or shorter (27 + oc_ prefix)
536-
* - Column names must be 30 chars or shorter
537-
* - Index names must be 30 chars or shorter
538-
* - Sequence names must be 30 chars or shorter
539-
* - Primary key names must be set or the table name 23 chars or shorter
540+
* - Tables names must be 63 chars or shorter (including its prefix (default 'oc_'))
541+
* - Column names must be 63 chars or shorter
542+
* - Index names must be 63 chars or shorter
543+
* - Sequence names must be 63 chars or shorter
544+
* - Primary key names must be set to 63 chars or shorter - or the table name must be <= 58 characters (63 - 5 for '_pKey' suffix) including the table name prefix
545+
*
546+
* This is based on the identifier limits set by our supported database engines:
547+
* - MySQL and MariaDB support 64 characters
548+
* - Oracle supports 128 characters (since 12c)
549+
* - PostgreSQL support 63
550+
* - SQLite does not have any limits
551+
*
552+
* @see https://github.com/nextcloud/documentation/blob/master/developer_manual/basics/storage/database.rst
553+
*
554+
* @throws \Doctrine\DBAL\Exception
555+
*/
556+
public function ensureNamingConstraints(Schema $sourceSchema, Schema $targetSchema, int $prefixLength): void {
557+
$MAX_NAME_LENGTH = 63;
558+
$sequences = $targetSchema->getSequences();
559+
560+
foreach ($targetSchema->getTables() as $table) {
561+
try {
562+
$sourceTable = $sourceSchema->getTable($table->getName());
563+
} catch (SchemaException $e) {
564+
// we only validate new tables
565+
if (\strlen($table->getName()) + $prefixLength > $MAX_NAME_LENGTH) {
566+
throw new \InvalidArgumentException('Table name "' . $table->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH);
567+
}
568+
$sourceTable = null;
569+
}
570+
571+
foreach ($table->getColumns() as $thing) {
572+
// If the table doesn't exist OR if the column doesn't exist in the table
573+
if ((!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName()))
574+
&& \strlen($thing->getName()) > $MAX_NAME_LENGTH
575+
) {
576+
throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH);
577+
}
578+
}
579+
580+
foreach ($table->getIndexes() as $thing) {
581+
if ((!$sourceTable instanceof Table || !$sourceTable->hasIndex($thing->getName()))
582+
&& \strlen($thing->getName()) > $MAX_NAME_LENGTH
583+
) {
584+
throw new \InvalidArgumentException('Index name "' . $table->getName() . '"."' . $thing->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH);
585+
}
586+
}
587+
588+
foreach ($table->getForeignKeys() as $thing) {
589+
if ((!$sourceTable instanceof Table || !$sourceTable->hasForeignKey($thing->getName()))
590+
&& \strlen($thing->getName()) > $MAX_NAME_LENGTH
591+
) {
592+
throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH);
593+
}
594+
}
595+
596+
$primaryKey = $table->getPrimaryKey();
597+
// only check if there is a primary key
598+
// and there was non in the old table or there was no old table
599+
if ($primaryKey !== null && ($sourceTable === null || $sourceTable->getPrimaryKey() === null)) {
600+
$indexName = strtolower($primaryKey->getName());
601+
$isUsingDefaultName = $indexName === 'primary';
602+
// This is the default name when using postgres - we use this for length comparison
603+
// as this is the longest default names for the DB engines provided by doctrine
604+
$defaultName = strtolower($table->getName() . '_pkey');
605+
606+
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_POSTGRES) {
607+
$isUsingDefaultName = $defaultName === $indexName;
608+
609+
if ($isUsingDefaultName) {
610+
$sequenceName = $table->getName() . '_' . implode('_', $primaryKey->getColumns()) . '_seq';
611+
$sequences = array_filter($sequences, function (Sequence $sequence) use ($sequenceName) {
612+
return $sequence->getName() !== $sequenceName;
613+
});
614+
}
615+
} elseif ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) {
616+
$isUsingDefaultName = strtolower($table->getName() . '_seq') === $indexName;
617+
}
618+
619+
if (!$isUsingDefaultName && \strlen($indexName) > $MAX_NAME_LENGTH) {
620+
throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH);
621+
}
622+
if ($isUsingDefaultName && \strlen($defaultName) + $prefixLength > $MAX_NAME_LENGTH) {
623+
throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH);
624+
}
625+
}
626+
}
627+
628+
foreach ($sequences as $sequence) {
629+
if (!$sourceSchema->hasSequence($sequence->getName())
630+
&& \strlen($sequence->getName()) > $MAX_NAME_LENGTH
631+
) {
632+
throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" exceeds the maximum length of ' . $MAX_NAME_LENGTH);
633+
}
634+
}
635+
}
636+
637+
/**
638+
* Enforces some data conventions to make sure tables can be used on Oracle SQL.
540639
*
541640
* Data constraints:
542641
* - Tables need a primary key (Not specific to Oracle, but required for performant clustering support)
@@ -546,66 +645,47 @@ public function executeStep($version, $schemaOnly = false) {
546645
* - Columns with type "string" can not be longer than 4.000 characters, use "text" instead
547646
*
548647
* @see https://github.com/nextcloud/documentation/blob/master/developer_manual/basics/storage/database.rst
549-
*
550-
* @param Schema $sourceSchema
551-
* @param Schema $targetSchema
552-
* @param int $prefixLength
553648
* @throws \Doctrine\DBAL\Exception
554649
*/
555-
public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) {
650+
public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSchema): void {
556651
$sequences = $targetSchema->getSequences();
557652

558653
foreach ($targetSchema->getTables() as $table) {
559654
try {
560655
$sourceTable = $sourceSchema->getTable($table->getName());
561656
} catch (SchemaException $e) {
562-
if (\strlen($table->getName()) - $prefixLength > 27) {
563-
throw new \InvalidArgumentException('Table name "' . $table->getName() . '" is too long.');
564-
}
565657
$sourceTable = null;
566658
}
567659

568-
foreach ($table->getColumns() as $thing) {
660+
foreach ($table->getColumns() as $column) {
569661
// If the table doesn't exist OR if the column doesn't exist in the table
570-
if (!$sourceTable instanceof Table || !$sourceTable->hasColumn($thing->getName())) {
571-
if (\strlen($thing->getName()) > 30) {
572-
throw new \InvalidArgumentException('Column name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
573-
}
574-
575-
if ($thing->getNotnull() && $thing->getDefault() === ''
576-
&& $sourceTable instanceof Table && !$sourceTable->hasColumn($thing->getName())) {
577-
throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is NotNull, but has empty string or null as default.');
662+
if (!$sourceTable instanceof Table || !$sourceTable->hasColumn($column->getName())) {
663+
if ($column->getNotnull() && $column->getDefault() === ''
664+
&& $sourceTable instanceof Table && !$sourceTable->hasColumn($column->getName())) {
665+
// null and empty string are the same on Oracle SQL
666+
throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $column->getName() . '" is NotNull, but has empty string or null as default.');
578667
}
579668

580-
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) {
669+
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE
670+
&& $column->getNotnull()
671+
&& Type::lookupName($column->getType()) === Types::BOOLEAN
672+
) {
581673
// Oracle doesn't support boolean column with non-null value
582-
if ($thing->getNotnull() && Type::lookupName($thing->getType()) === Types::BOOLEAN) {
583-
$thing->setNotnull(false);
584-
}
674+
// to still allow lighter DB schemas on other providers we force it to not null
675+
// see https://github.com/nextcloud/server/pull/55156
676+
$column->setNotnull(false);
585677
}
586678

587679
$sourceColumn = null;
588680
} else {
589-
$sourceColumn = $sourceTable->getColumn($thing->getName());
681+
$sourceColumn = $sourceTable->getColumn($column->getName());
590682
}
591683

592684
// If the column was just created OR the length changed OR the type changed
593685
// we will NOT detect invalid length if the column is not modified
594-
if (($sourceColumn === null || $sourceColumn->getLength() !== $thing->getLength() || Type::lookupName($sourceColumn->getType()) !== Types::STRING)
595-
&& $thing->getLength() > 4000 && Type::lookupName($thing->getType()) === Types::STRING) {
596-
throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $thing->getName() . '" is type String, but exceeding the 4.000 length limit.');
597-
}
598-
}
599-
600-
foreach ($table->getIndexes() as $thing) {
601-
if ((!$sourceTable instanceof Table || !$sourceTable->hasIndex($thing->getName())) && \strlen($thing->getName()) > 30) {
602-
throw new \InvalidArgumentException('Index name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
603-
}
604-
}
605-
606-
foreach ($table->getForeignKeys() as $thing) {
607-
if ((!$sourceTable instanceof Table || !$sourceTable->hasForeignKey($thing->getName())) && \strlen($thing->getName()) > 30) {
608-
throw new \InvalidArgumentException('Foreign key name "' . $table->getName() . '"."' . $thing->getName() . '" is too long.');
686+
if (($sourceColumn === null || $sourceColumn->getLength() !== $column->getLength() || Type::lookupName($sourceColumn->getType()) !== Types::STRING)
687+
&& $column->getLength() > 4000 && Type::lookupName($column->getType()) === Types::STRING) {
688+
throw new \InvalidArgumentException('Column "' . $table->getName() . '"."' . $column->getName() . '" is type String, but exceeding the 4.000 length limit.');
609689
}
610690
}
611691

@@ -628,26 +708,13 @@ public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSche
628708
$defaultName = $table->getName() . '_seq';
629709
$isUsingDefaultName = strtolower($defaultName) === $indexName;
630710
}
631-
632-
if (!$isUsingDefaultName && \strlen($indexName) > 30) {
633-
throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" is too long.');
634-
}
635-
if ($isUsingDefaultName && \strlen($table->getName()) - $prefixLength >= 23) {
636-
throw new \InvalidArgumentException('Primary index name on "' . $table->getName() . '" is too long.');
637-
}
638711
} elseif (!$primaryKey instanceof Index && !$sourceTable instanceof Table) {
639712
/** @var LoggerInterface $logger */
640-
$logger = \OC::$server->get(LoggerInterface::class);
713+
$logger = \OCP\Server::get(LoggerInterface::class);
641714
$logger->error('Table "' . $table->getName() . '" has no primary key and therefor will not behave sane in clustered setups. This will throw an exception and not be installable in a future version of Nextcloud.');
642715
// throw new \InvalidArgumentException('Table "' . $table->getName() . '" has no primary key and therefor will not behave sane in clustered setups.');
643716
}
644717
}
645-
646-
foreach ($sequences as $sequence) {
647-
if (!$sourceSchema->hasSequence($sequence->getName()) && \strlen($sequence->getName()) > 30) {
648-
throw new \InvalidArgumentException('Sequence name "' . $sequence->getName() . '" is too long.');
649-
}
650-
}
651718
}
652719

653720
/**

0 commit comments

Comments
 (0)