From 1751f2b60f7b4ea879823c61c1c6de591feb8c9c Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 1 Sep 2025 10:51:14 +0300 Subject: [PATCH 1/2] Fix database command warnings --- app/src/Lib/Util/SchemaManager.php | 91 ++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 10 deletions(-) diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index 528ef2bd0..cb802c1ed 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -327,15 +327,17 @@ protected function processSchema( } // This is the SQL that represents the desired state of the database - $toSql = $schema->toSql($this->conn->getDatabasePlatform()); + // XXX This never used keeping as a reference + // $toSql = $schema->toSql($this->conn->getDatabasePlatform()); // SchemaManager provides info about the database $sm = $this->conn->createSchemaManager(); - // The is the current database representation - $curSchema = $sm->createSchema(); - - $fromSql = $curSchema->toSql($this->conn->getDatabasePlatform()); + // This is the current database representation + $curSchema = $sm->introspectSchema(); + + // XXX This is never used keeping as a reference + // $fromSql = $curSchema->toSql($this->conn->getDatabasePlatform()); try { // We manually call compare so we can get the SchemaDiff object. We need @@ -359,12 +361,43 @@ protected function processSchema( // Remove the DROP SEQUENCE statements in $fromSql because they're Postgres automagic // being misinterpreted. (Note toSaveSql might mask this now.) // XXX Maybe debug and file a PR to not emit DROP SEQUENCE on PG for autoincrementesque fields? - if($this->io) $io->out("Skipping sequence drop"); - } else { + if($this->io) $this->io->out("Skipping sequence drop"); + continue; + } + + // PostgreSQL: translate DROP INDEX for names that are actually unique constraints + if($this->conn->driver == 'Cake\Database\Driver\Postgres' + && preg_match('/^DROP INDEX\s+"?([a-zA-Z0-9_\.]+)"?/i', $sql, $m)) { + $idxOrConstraint = $m[1]; + + // Resolve if this name is a constraint and on which table + $qualifiedTable = $this->resolvePgConstraintTable($idxOrConstraint); + + if($qualifiedTable !== null) { + // We will skip this index if it's bound to a constraint + if($this->io) $this->io->out('Skipping index drop since it is bound to a constraint'); + continue; + } + } + + // PostgreSQL: make index renames idempotent and tolerant using IF EXISTS + if($this->conn->driver == 'Cake\Database\Driver\Postgres' + && preg_match('/^ALTER INDEX\s+([^\s]+)\s+RENAME\s+TO\s+([^\s]+)$/i', trim($sql), $m)) { + $oldName = $m[1]; + $newName = $m[2]; + + // Use IF EXISTS to avoid error if the old index name doesn't exist in this environment + $translated = 'ALTER INDEX IF EXISTS ' . $oldName . ' RENAME TO ' . $newName; + if($this->io) $this->io->out($translated . ' -- added IF EXISTS for PostgreSQL resilience'); if(!$diffOnly) { - $stmt = $this->conn->executeQuery($sql); - // $stmt just returns the query string so we don't bother examining it + $this->conn->executeQuery($translated); } + continue; + } + + if(!$diffOnly) { + $stmt = $this->conn->executeQuery($sql); + // $stmt just returns the query string so we don't bother examining it } } @@ -380,4 +413,42 @@ protected function processSchema( // but so far we don't have an example indicating it's needed. } -} \ No newline at end of file + /** + * Resolve a PostgreSQL constraint's owning table (schema-qualified) by constraint name. + * Returns null if the given name is not found as a constraint. + * + * @param string $constraintName + * @return ?string Schema-qualified table name or null if constraint not found + * @since COmanage Registry v5.2.0 + */ + + protected function resolvePgConstraintTable(string $constraintName): ?string { + if($this->conn->driver !== 'Cake\\Database\\Driver\\Postgres') { + return null; + } + + // Query pg catalogs to find the table for this constraint name + // We return schema-qualified name: schema.table + $sql = <<conn->executeQuery($sql, ['name' => $constraintName]); + $row = $stmt->fetchAssociative(); + if(!$row || empty($row['schema']) || empty($row['table'])) { + return null; + } + // Quote/qualify as schema.table (matching how other statements look) + return $row['schema'] . '.' . $row['table']; + } catch(\Throwable $e) { + // On any error, fail open by returning null so caller uses original SQL + return null; + } + } +} From 8dcf207f1c67d19fe2d5222de7ff7ea43e06a578 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 1 Sep 2025 20:01:09 +0300 Subject: [PATCH 2/2] Replace deprecated saveToSql with custom method --- app/src/Lib/Util/SchemaManager.php | 63 ++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index cb802c1ed..77f1245f1 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -31,6 +31,8 @@ use Cake\Console\ConsoleIo; +use Doctrine\DBAL\Exception; +use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\Schema; use Doctrine\DBAL\Schema\SchemaDiff; @@ -346,8 +348,11 @@ protected function processSchema( // schema file). $comparator = new Comparator(); $schemaDiff = $comparator->compareSchemas($curSchema, $schema); - - $diffSql = $schemaDiff->toSaveSql($this->conn->getDatabasePlatform()); + + $diffSql = $this->getAlterSchemaSqlSaveMode( + $this->conn->getDatabasePlatform(), + $schemaDiff + ); // We don't start a transaction since in general we always want to move to // the desired state, and if we fail in flight it's probably a bug that @@ -375,7 +380,7 @@ protected function processSchema( if($qualifiedTable !== null) { // We will skip this index if it's bound to a constraint - if($this->io) $this->io->out('Skipping index drop since it is bound to a constraint'); + if($this->io) $this->io->out('Skipping index:' . $m[1] . ' drop since it is bound to a constraint'); continue; } } @@ -451,4 +456,54 @@ protected function resolvePgConstraintTable(string $constraintName): ?string { return null; } } -} + + + /** + * Generate non-destructive (save-mode) schema SQL. + * + * Equivalent to SchemaDiff::_toSql($platform, true). + * + * - Does NOT drop tables + * - Does NOT drop sequences + * - Does NOT drop orphaned FKs + * + * @param AbstractPlatform $platform Database platform abstraction + * @param SchemaDiff $diff Schema differences to generate SQL for + * @return array Array of SQL statements to execute + * @throws Exception + * @since COmanage Registry v5.2.0 + */ + protected function getAlterSchemaSqlSaveMode( + AbstractPlatform $platform, + SchemaDiff $diff + ): array { + $sql = []; + + // 1) Schemas to create + if ($platform->supportsSchemas()) { + foreach ($diff->getCreatedSchemas() as $schema) { + $sql[] = $platform->getCreateSchemaSQL($schema); + } + } + + // 2) Sequences (alter + create; no drops in save mode) + if ($platform->supportsSequences()) { + foreach ($diff->getAlteredSequences() as $sequence) { + $sql[] = $platform->getAlterSequenceSQL($sequence); + } + foreach ($diff->getCreatedSequences() as $sequence) { + $sql[] = $platform->getCreateSequenceSQL($sequence); + } + } + + // 3) Tables (create only; no drops in save mode) + $sql = array_merge($sql, $platform->getCreateTablesSQL($diff->getCreatedTables())); + + // 4) Alter existing tables (may emit FK drops/adds as required for alterations) + foreach ($diff->getAlteredTables() as $tableDiff) { + $sql = array_merge($sql, $platform->getAlterTableSQL($tableDiff)); + } + + return $sql; + } +} \ No newline at end of file