From 18f3c7a2bc22184dfd2d0cff925842bca7d5b40e Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 00:13:21 -0500 Subject: [PATCH 01/12] Improve test run without optional connections --- tests/TestCase/Command/BakeMigrationDiffCommandTest.php | 8 +++++--- tests/TestCase/Migration/EnvironmentTest.php | 2 +- tests/TestCase/MigrationsTest.php | 2 ++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php index a7362740..0de89739 100644 --- a/tests/TestCase/Command/BakeMigrationDiffCommandTest.php +++ b/tests/TestCase/Command/BakeMigrationDiffCommandTest.php @@ -170,7 +170,7 @@ public function testBakingDiffAddRemove() */ public function testBakingDiffWithAutoIdCompatibleSignedPrimaryKeys(): void { - $this->skipIf(getenv('DB_URL_COMPARE') === false); + $this->skipIf(!env('DB_URL_COMPARE')); Configure::write('Migrations.unsigned_primary_keys', false); @@ -183,7 +183,7 @@ public function testBakingDiffWithAutoIdCompatibleSignedPrimaryKeys(): void */ public function testBakingDiffWithAutoIdIncompatibleSignedPrimaryKeys(): void { - $this->skipIf(getenv('DB_URL_COMPARE') === false); + $this->skipIf(!env('DB_URL_COMPARE')); $this->runDiffBakingTest('WithAutoIdIncompatibleSignedPrimaryKeys'); } @@ -194,7 +194,7 @@ public function testBakingDiffWithAutoIdIncompatibleSignedPrimaryKeys(): void */ public function testBakingDiffWithAutoIdIncompatibleUnsignedPrimaryKeys(): void { - $this->skipIf(getenv('DB_URL_COMPARE') === false); + $this->skipIf(!env('DB_URL_COMPARE')); Configure::write('Migrations.unsigned_primary_keys', false); @@ -203,6 +203,8 @@ public function testBakingDiffWithAutoIdIncompatibleUnsignedPrimaryKeys(): void protected function runDiffBakingTest(string $scenario): void { + $this->skipIf(!env('DB_URL_COMPARE')); + $diffConfigFolder = Plugin::path('Migrations') . 'tests' . DS . 'comparisons' . DS . 'Diff' . DS . lcfirst($scenario) . DS; $diffMigrationsPath = $diffConfigFolder . 'the_diff_' . Inflector::underscore($scenario) . '_' . env('DB') . '.php'; $diffDumpPath = $diffConfigFolder . 'schema-dump-test_comparisons_' . env('DB') . '.lock'; diff --git a/tests/TestCase/Migration/EnvironmentTest.php b/tests/TestCase/Migration/EnvironmentTest.php index 7e79a780..7863dd34 100644 --- a/tests/TestCase/Migration/EnvironmentTest.php +++ b/tests/TestCase/Migration/EnvironmentTest.php @@ -1,7 +1,7 @@ markTestSkipped('Incompatible with sqlserver right now.'); } + $snapshotConfig = ConnectionManager::getConfig('test_snapshot'); + $this->skipIf(empty($snapshotConfig['database']), 'Requires test_snapshot connection'); if ($flags) { Configure::write('Migrations', $flags + Configure::read('Migrations', [])); From 36b53f3df9e2e92f3182931fd988f7f83b253312 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 00:26:40 -0500 Subject: [PATCH 02/12] Start removing sprintf from PdoAdapter --- src/Db/Adapter/PdoAdapter.php | 38 ++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/Db/Adapter/PdoAdapter.php b/src/Db/Adapter/PdoAdapter.php index b1d4452d..5c1eead9 100644 --- a/src/Db/Adapter/PdoAdapter.php +++ b/src/Db/Adapter/PdoAdapter.php @@ -479,31 +479,33 @@ public function migrated(MigrationInterface $migration, string $direction, strin if (strcasecmp($direction, MigrationInterface::UP) === 0) { // up $sql = sprintf( - "INSERT INTO %s (%s, %s, %s, %s, %s) VALUES ('%s', '%s', '%s', '%s', %s);", + "INSERT INTO %s (%s, %s, %s, %s, %s) VALUES (?, ?, ?, ?, ?);", $this->quoteTableName($this->getSchemaTableName()), $this->quoteColumnName('version'), $this->quoteColumnName('migration_name'), $this->quoteColumnName('start_time'), $this->quoteColumnName('end_time'), $this->quoteColumnName('breakpoint'), + ); + $params = [ $migration->getVersion(), substr($migration->getName(), 0, 100), $startTime, $endTime, - $this->castToBool(false) - ); + $this->castToBool(false), + ]; - $this->execute($sql); + $this->execute($sql, $params); } else { // down $sql = sprintf( - "DELETE FROM %s WHERE %s = '%s'", + 'DELETE FROM %s WHERE %s = ?', $this->quoteTableName($this->getSchemaTableName()), $this->quoteColumnName('version'), - $migration->getVersion() ); + $params = [$migration->getVersion()]; - $this->execute($sql); + $this->execute($sql, $params); } return $this; @@ -514,17 +516,18 @@ public function migrated(MigrationInterface $migration, string $direction, strin */ public function toggleBreakpoint(MigrationInterface $migration): AdapterInterface { + $params = [ + $migration->getVersion(), + ]; $this->query( sprintf( - 'UPDATE %1$s SET %2$s = CASE %2$s WHEN %3$s THEN %4$s ELSE %3$s END, %7$s = %7$s WHERE %5$s = \'%6$s\';', + 'UPDATE %1$s SET %2$s = CASE %2$s WHEN true THEN false ELSE true END, %4$s = %4$s WHERE %3$s = ?;', $this->quoteTableName($this->getSchemaTableName()), $this->quoteColumnName('breakpoint'), - $this->castToBool(true), - $this->castToBool(false), $this->quoteColumnName('version'), - $migration->getVersion(), $this->quoteColumnName('start_time') - ) + ), + $params ); return $this; @@ -575,16 +578,19 @@ public function unsetBreakpoint(MigrationInterface $migration): AdapterInterface */ protected function markBreakpoint(MigrationInterface $migration, bool $state): AdapterInterface { + $params = [ + $this->castToBool($state), + $migration->getVersion() + ]; $this->query( sprintf( - 'UPDATE %1$s SET %2$s = %3$s, %4$s = %4$s WHERE %5$s = \'%6$s\';', + 'UPDATE %1$s SET %2$s = ?, %3$s = %3$s WHERE %4$s = ?;', $this->quoteTableName($this->getSchemaTableName()), $this->quoteColumnName('breakpoint'), - $this->castToBool($state), $this->quoteColumnName('start_time'), $this->quoteColumnName('version'), - $migration->getVersion() - ) + ), + $params ); return $this; From ba8657ec29c74fafa93580c2faa98b6b08fd4433 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 01:58:15 -0500 Subject: [PATCH 03/12] Use prepared statements in postgres adapter --- src/Db/Adapter/PdoAdapter.php | 2 +- src/Db/Adapter/PostgresAdapter.php | 54 +++++++++++++++--------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/Db/Adapter/PdoAdapter.php b/src/Db/Adapter/PdoAdapter.php index 5c1eead9..e878ae66 100644 --- a/src/Db/Adapter/PdoAdapter.php +++ b/src/Db/Adapter/PdoAdapter.php @@ -580,7 +580,7 @@ protected function markBreakpoint(MigrationInterface $migration, bool $state): A { $params = [ $this->castToBool($state), - $migration->getVersion() + $migration->getVersion(), ]; $this->query( sprintf( diff --git a/src/Db/Adapter/PostgresAdapter.php b/src/Db/Adapter/PostgresAdapter.php index a525c219..ee67d88b 100644 --- a/src/Db/Adapter/PostgresAdapter.php +++ b/src/Db/Adapter/PostgresAdapter.php @@ -496,16 +496,15 @@ protected function getRenameColumnInstructions( string $newColumnName ): AlterInstructions { $parts = $this->getSchemaName($tableName); - $sql = sprintf( - 'SELECT CASE WHEN COUNT(*) > 0 THEN 1 ELSE 0 END AS column_exists + $sql = 'SELECT CASE WHEN COUNT(*) > 0 THEN 1 ELSE 0 END AS column_exists FROM information_schema.columns - WHERE table_schema = %s AND table_name = %s AND column_name = %s', - $this->quoteString($parts['schema']), - $this->quoteString($parts['table']), - $this->quoteString($columnName) - ); - - $result = $this->fetchRow($sql); + WHERE table_schema = ? AND table_name = ? AND column_name = ?'; + $params = [ + $parts['schema'], + $parts['table'], + $columnName, + ]; + $result = $this->query($sql, $params)->fetch('assoc'); if (!$result || !(bool)$result['column_exists']) { throw new InvalidArgumentException("The specified column does not exist: $columnName"); } @@ -833,7 +832,11 @@ public function hasPrimaryKey(string $tableName, $columns, ?string $constraint = public function getPrimaryKey(string $tableName): array { $parts = $this->getSchemaName($tableName); - $rows = $this->fetchAll(sprintf( + $params = [ + $parts['schema'], + $parts['table'], + ]; + $rows = $this->query( "SELECT tc.constraint_name, kcu.column_name @@ -841,12 +844,11 @@ public function getPrimaryKey(string $tableName): array JOIN information_schema.key_column_usage AS kcu ON tc.constraint_name = kcu.constraint_name WHERE constraint_type = 'PRIMARY KEY' - AND tc.table_schema = %s - AND tc.table_name = %s + AND tc.table_schema = ? + AND tc.table_name = ? ORDER BY kcu.position_in_unique_constraint", - $this->quoteString($parts['schema']), - $this->quoteString($parts['table']) - )); + $params, + )->fetchAll('assoc'); $primaryKey = [ 'columns' => [], @@ -896,7 +898,11 @@ protected function getForeignKeys(string $tableName): array { $parts = $this->getSchemaName($tableName); $foreignKeys = []; - $rows = $this->fetchAll(sprintf( + $params = [ + $parts['schema'], + $parts['table'], + ]; + $rows = $this->query( "SELECT tc.constraint_name, tc.table_name, kcu.column_name, @@ -906,11 +912,10 @@ protected function getForeignKeys(string $tableName): array information_schema.table_constraints AS tc JOIN information_schema.key_column_usage AS kcu ON tc.constraint_name = kcu.constraint_name JOIN information_schema.constraint_column_usage AS ccu ON ccu.constraint_name = tc.constraint_name - WHERE constraint_type = 'FOREIGN KEY' AND tc.table_schema = %s AND tc.table_name = %s + WHERE constraint_type = 'FOREIGN KEY' AND tc.table_schema = ? AND tc.table_name = ? ORDER BY kcu.ordinal_position", - $this->quoteString($parts['schema']), - $this->quoteString($parts['table']) - )); + $params, + )->fetchAll('assoc'); foreach ($rows as $row) { $foreignKeys[$row['constraint_name']]['table'] = $row['table_name']; $foreignKeys[$row['constraint_name']]['columns'][] = $row['column_name']; @@ -1384,13 +1389,8 @@ public function createSchema(string $schemaName = 'public'): void */ public function hasSchema(string $schemaName): bool { - $sql = sprintf( - 'SELECT count(*) - FROM pg_namespace - WHERE nspname = %s', - $this->quoteString($schemaName) - ); - $result = $this->fetchRow($sql); + $sql = 'SELECT count(*) FROM pg_namespace WHERE nspname = ?'; + $result = $this->query($sql, [$schemaName])->fetch('assoc'); if (!$result) { return false; } From 17cec155608c26a6e5d621ef048f1a94f0d63231 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 12:20:22 -0500 Subject: [PATCH 04/12] Remove more sprintf from adapters When possible we should use prepared statements. --- src/Db/Adapter/MysqlAdapter.php | 51 ++++++------ src/Db/Adapter/SqliteAdapter.php | 30 +++---- src/Db/Adapter/SqlserverAdapter.php | 119 ++++++++++++---------------- 3 files changed, 92 insertions(+), 108 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 359ec179..ce6da2dd 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -751,19 +751,22 @@ public function hasPrimaryKey(string $tableName, $columns, ?string $constraint = public function getPrimaryKey(string $tableName): array { $options = $this->getOptions(); - $rows = $this->fetchAll(sprintf( + $params = [ + $options['database'], + $tableName, + ]; + $rows = $this->query( "SELECT k.CONSTRAINT_NAME, k.COLUMN_NAME FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS t JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE k USING(CONSTRAINT_NAME,TABLE_SCHEMA,TABLE_NAME) - WHERE t.CONSTRAINT_TYPE='PRIMARY KEY' - AND t.TABLE_SCHEMA='%s' - AND t.TABLE_NAME='%s'", - $options['database'], - $tableName - )); + WHERE t.CONSTRAINT_TYPE = 'PRIMARY KEY' + AND t.TABLE_SCHEMA = ? + AND t.TABLE_NAME = ?", + $params + )->fetchAll('assoc'); $primaryKey = [ 'columns' => [], @@ -814,7 +817,11 @@ protected function getForeignKeys(string $tableName): array } $foreignKeys = []; - $rows = $this->fetchAll(sprintf( + $params = [ + empty($schema) ? 'DATABASE()' : "'$schema'", + $tableName, + ]; + $rows = $this->query( "SELECT CONSTRAINT_NAME, CONCAT(TABLE_SCHEMA, '.', TABLE_NAME) AS TABLE_NAME, @@ -826,9 +833,8 @@ protected function getForeignKeys(string $tableName): array AND TABLE_SCHEMA = %s AND TABLE_NAME = '%s' ORDER BY POSITION_IN_UNIQUE_CONSTRAINT", - empty($schema) ? 'DATABASE()' : "'$schema'", - $tableName - )); + $params + )->fetchAll('assoc'); foreach ($rows as $row) { $foreignKeys[$row['CONSTRAINT_NAME']]['table'] = $row['TABLE_NAME']; $foreignKeys[$row['CONSTRAINT_NAME']]['columns'][] = $row['COLUMN_NAME']; @@ -1270,12 +1276,10 @@ public function createDatabase(string $name, array $options = []): void */ public function hasDatabase(string $name): bool { - $rows = $this->fetchAll( - sprintf( - 'SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = \'%s\'', - $name - ) - ); + $rows = $this->query( + 'SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = ?', + [$name] + )->fetchAll('assoc'); foreach ($rows as $row) { if (!empty($row)) { @@ -1470,16 +1474,13 @@ public function describeTable(string $tableName): array $options = $this->getOptions(); // mysql specific - $sql = sprintf( - "SELECT * + $sql = "SELECT * FROM information_schema.tables - WHERE table_schema = '%s' - AND table_name = '%s'", - $options['database'], - $tableName - ); + WHERE table_schema = ? + AND table_name = ?"; + $params = [$options['database'], $tableName]; - $table = $this->fetchRow($sql); + $table = $this->query($sql, $params)->fetch('assoc'); return $table !== false ? $table : []; } diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index 540b675f..58020966 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -327,7 +327,11 @@ protected function resolveTable(string $tableName): array $master = sprintf('%s.%s', $this->quoteColumnName($schema), 'sqlite_master'); } try { - $rows = $this->fetchAll(sprintf("SELECT name FROM %s WHERE type='table' AND lower(name) = %s", $master, $this->quoteString($table))); + $params = [$table]; + $rows = $this->query( + "SELECT name FROM {$master} WHERE type = 'table' AND lower(name) = ?", + $params + )->fetchAll('assoc'); } catch (PDOException $e) { // an exception can occur if the schema part of the table refers to a database which is not attached break; @@ -797,19 +801,17 @@ protected function bufferIndicesAndTriggers(AlterInstructions $instructions, str $state['indices'] = []; $state['triggers'] = []; - $rows = $this->fetchAll( - sprintf( - " - SELECT * - FROM sqlite_master - WHERE - (`type` = 'index' OR `type` = 'trigger') - AND tbl_name = %s - AND sql IS NOT NULL - ", - $this->quoteValue($tableName) - ) - ); + $params = [$tableName]; + $rows = $this->query( + "SELECT * + FROM sqlite_master + WHERE + (`type` = 'index' OR `type` = 'trigger') + AND tbl_name = ? + AND sql IS NOT NULL + ", + $params + )->fetchAll('assoc'); $schema = $this->getSchemaName($tableName, true)['schema']; diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index 0e3b43c0..6f248fb2 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -124,7 +124,10 @@ public function hasTable(string $tableName): bool } /** @var array $result */ - $result = $this->fetchRow(sprintf("SELECT count(*) as [count] FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = '%s';", $tableName)); + $result = $this->query( + "SELECT count(*) as [count] FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = ?", + [$tableName] + )->fetch('assoc'); return $result['count'] > 0; } @@ -322,7 +325,7 @@ public function truncateTable(string $tableName): void */ public function getColumnComment(string $tableName, ?string $columnName): ?string { - $sql = sprintf("SELECT cast(extended_properties.[value] as nvarchar(4000)) comment + $sql = "SELECT cast(extended_properties.[value] as nvarchar(4000)) comment FROM sys.schemas INNER JOIN sys.tables ON schemas.schema_id = tables.schema_id @@ -332,8 +335,9 @@ public function getColumnComment(string $tableName, ?string $columnName): ?strin ON tables.object_id = extended_properties.major_id AND columns.column_id = extended_properties.minor_id AND extended_properties.name = 'MS_Description' - WHERE schemas.[name] = '%s' AND tables.[name] = '%s' AND columns.[name] = '%s'", $this->schema, $tableName, (string)$columnName); - $row = $this->fetchRow($sql); + WHERE schemas.[name] = ? AND tables.[name] = ? AND columns.[name] = ?"; + $params = [ $this->schema, $tableName, (string)$columnName]; + $row = $this->query($sql, $params)->fetch('assoc'); if ($row) { return trim($row['comment']); @@ -348,19 +352,16 @@ public function getColumnComment(string $tableName, ?string $columnName): ?strin public function getColumns(string $tableName): array { $columns = []; - $sql = sprintf( - "SELECT DISTINCT TABLE_SCHEMA AS [schema], TABLE_NAME as [table_name], COLUMN_NAME AS [name], DATA_TYPE AS [type], + $sql = "SELECT DISTINCT TABLE_SCHEMA AS [schema], TABLE_NAME as [table_name], COLUMN_NAME AS [name], DATA_TYPE AS [type], IS_NULLABLE AS [null], COLUMN_DEFAULT AS [default], CHARACTER_MAXIMUM_LENGTH AS [char_length], NUMERIC_PRECISION AS [precision], NUMERIC_SCALE AS [scale], ORDINAL_POSITION AS [ordinal_position], COLUMNPROPERTY(object_id(TABLE_NAME), COLUMN_NAME, 'IsIdentity') as [identity] FROM INFORMATION_SCHEMA.COLUMNS - WHERE TABLE_NAME = '%s' - ORDER BY ordinal_position", - $tableName - ); - $rows = $this->fetchAll($sql); + WHERE TABLE_NAME = ? + ORDER BY ordinal_position"; + $rows = $this->query($sql, [$tableName])->fetchAll('assoc'); foreach ($rows as $columnInfo) { try { $type = $this->getPhinxType($columnInfo['type']); @@ -415,15 +416,11 @@ protected function parseDefault(?string $default): int|string|null */ public function hasColumn(string $tableName, string $columnName): bool { - $sql = sprintf( - "SELECT count(*) as [count] + $sql = "SELECT count(*) as [count] FROM INFORMATION_SCHEMA.COLUMNS - WHERE TABLE_NAME = '%s' AND COLUMN_NAME = '%s'", - $tableName, - $columnName - ); + WHERE TABLE_NAME = ? AND COLUMN_NAME = ?"; /** @var array $result */ - $result = $this->fetchRow($sql); + $result = $this->query($sql, [$tableName, $columnName])->fetch('assoc'); return $result['count'] > 0; } @@ -593,29 +590,14 @@ protected function getDropDefaultConstraint(string $tableName, ?string $columnNa */ protected function getDefaultConstraint(string $tableName, string $columnName): string|false { - $sql = "SELECT - default_constraints.name -FROM - sys.all_columns - - INNER JOIN - sys.tables - ON all_columns.object_id = tables.object_id - - INNER JOIN - sys.schemas - ON tables.schema_id = schemas.schema_id + $sql = "SELECT default_constraints.name + FROM sys.all_columns + INNER JOIN sys.tables ON all_columns.object_id = tables.object_id + INNER JOIN sys.schemas ON tables.schema_id = schemas.schema_id + INNER JOIN sys.default_constraints ON all_columns.default_object_id = default_constraints.object_id + WHERE schemas.name = 'dbo' AND tables.name = ? AND all_columns.name = ?"; - INNER JOIN - sys.default_constraints - ON all_columns.default_object_id = default_constraints.object_id - -WHERE - schemas.name = 'dbo' - AND tables.name = '{$tableName}' - AND all_columns.name = '{$columnName}'"; - - $rows = $this->fetchAll($sql); + $rows = $this->query($sql, [$tableName, $columnName])->fetchAll('assoc'); return empty($rows) ? false : $rows[0]['name']; } @@ -627,13 +609,14 @@ protected function getDefaultConstraint(string $tableName, string $columnName): */ protected function getIndexColums(string $tableId, string $indexId): array { - $sql = "SELECT AC.[name] AS [column_name] + $sql = 'SELECT AC.[name] AS [column_name] FROM sys.[index_columns] IC INNER JOIN sys.[all_columns] AC ON IC.[column_id] = AC.[column_id] -WHERE AC.[object_id] = {$tableId} AND IC.[index_id] = {$indexId} AND IC.[object_id] = {$tableId} -ORDER BY IC.[key_ordinal];"; +WHERE AC.[object_id] = ? AND IC.[index_id] = ? AND IC.[object_id] = ? +ORDER BY IC.[key_ordinal]'; - $rows = $this->fetchAll($sql); + $params = [$tableId, $indexId, $tableId]; + $rows = $this->query($sql, $params)->fetchAll('assoc'); $columns = []; foreach ($rows as $row) { $columns[] = strtolower($row['column_name']); @@ -654,10 +637,10 @@ public function getIndexes(string $tableName): array $sql = "SELECT I.[name] AS [index_name], I.[index_id] as [index_id], T.[object_id] as [table_id] FROM sys.[tables] AS T INNER JOIN sys.[indexes] I ON T.[object_id] = I.[object_id] -WHERE T.[is_ms_shipped] = 0 AND I.[type_desc] <> 'HEAP' AND T.[name] = '{$tableName}' -ORDER BY T.[name], I.[index_id];"; +WHERE T.[is_ms_shipped] = 0 AND I.[type_desc] <> 'HEAP' AND T.[name] = ? +ORDER BY T.[name], I.[index_id]"; - $rows = $this->fetchAll($sql); + $rows = $this->query($sql, [$tableName])->fetchAll('assoc'); foreach ($rows as $row) { $columns = $this->getIndexColums($row['table_id'], $row['index_id']); $indexes[$row['index_name']] = ['columns' => $columns]; @@ -803,7 +786,7 @@ public function hasPrimaryKey(string $tableName, $columns, ?string $constraint = */ public function getPrimaryKey(string $tableName): array { - $rows = $this->fetchAll(sprintf( + $rows = $this->query( "SELECT tc.CONSTRAINT_NAME, kcu.COLUMN_NAME @@ -813,8 +796,8 @@ public function getPrimaryKey(string $tableName): array WHERE CONSTRAINT_TYPE = 'PRIMARY KEY' AND tc.TABLE_NAME = '%s' ORDER BY kcu.ORDINAL_POSITION", - $tableName - )); + [$tableName] + )->fetchAll('assoc'); $primaryKey = [ 'columns' => [], @@ -863,20 +846,20 @@ public function hasForeignKey(string $tableName, $columns, ?string $constraint = protected function getForeignKeys(string $tableName): array { $foreignKeys = []; - $rows = $this->fetchAll(sprintf( + $rows = $this->query( "SELECT - tc.CONSTRAINT_NAME, - tc.TABLE_NAME, kcu.COLUMN_NAME, - ccu.TABLE_NAME AS REFERENCED_TABLE_NAME, - ccu.COLUMN_NAME AS REFERENCED_COLUMN_NAME - FROM - INFORMATION_SCHEMA.TABLE_CONSTRAINTS AS tc - JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS kcu ON tc.CONSTRAINT_NAME = kcu.CONSTRAINT_NAME - JOIN INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE AS ccu ON ccu.CONSTRAINT_NAME = tc.CONSTRAINT_NAME - WHERE CONSTRAINT_TYPE = 'FOREIGN KEY' AND tc.TABLE_NAME = '%s' - ORDER BY kcu.ORDINAL_POSITION", - $tableName - )); + tc.CONSTRAINT_NAME, + tc.TABLE_NAME, kcu.COLUMN_NAME, + ccu.TABLE_NAME AS REFERENCED_TABLE_NAME, + ccu.COLUMN_NAME AS REFERENCED_COLUMN_NAME + FROM + INFORMATION_SCHEMA.TABLE_CONSTRAINTS AS tc + JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS kcu ON tc.CONSTRAINT_NAME = kcu.CONSTRAINT_NAME + JOIN INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE AS ccu ON ccu.CONSTRAINT_NAME = tc.CONSTRAINT_NAME + WHERE CONSTRAINT_TYPE = 'FOREIGN KEY' AND tc.TABLE_NAME = ? + ORDER BY kcu.ORDINAL_POSITION", + [$tableName] + )->fetchAll('assoc'); foreach ($rows as $row) { $foreignKeys[$row['CONSTRAINT_NAME']]['table'] = $row['TABLE_NAME']; $foreignKeys[$row['CONSTRAINT_NAME']]['columns'][] = $row['COLUMN_NAME']; @@ -1086,12 +1069,10 @@ public function createDatabase(string $name, array $options = []): void public function hasDatabase(string $name): bool { /** @var array $result */ - $result = $this->fetchRow( - sprintf( - "SELECT count(*) as [count] FROM master.dbo.sysdatabases WHERE [name] = '%s'", - $name - ) - ); + $result = $this->query( + "SELECT count(*) as [count] FROM master.dbo.sysdatabases WHERE [name] = ?", + [$name] + )->fetch('assoc'); return $result['count'] > 0; } From 714af3aa0cd263e7737a28414a4768670a84f0d5 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 12:29:30 -0500 Subject: [PATCH 05/12] Fix more queries from 4.next --- src/Db/Adapter/PdoAdapter.php | 2 +- src/Db/Adapter/SqlserverAdapter.php | 15 +++++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/Db/Adapter/PdoAdapter.php b/src/Db/Adapter/PdoAdapter.php index e878ae66..e5d0c068 100644 --- a/src/Db/Adapter/PdoAdapter.php +++ b/src/Db/Adapter/PdoAdapter.php @@ -479,7 +479,7 @@ public function migrated(MigrationInterface $migration, string $direction, strin if (strcasecmp($direction, MigrationInterface::UP) === 0) { // up $sql = sprintf( - "INSERT INTO %s (%s, %s, %s, %s, %s) VALUES (?, ?, ?, ?, ?);", + 'INSERT INTO %s (%s, %s, %s, %s, %s) VALUES (?, ?, ?, ?, ?);', $this->quoteTableName($this->getSchemaTableName()), $this->quoteColumnName('version'), $this->quoteColumnName('migration_name'), diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index d45f19c6..2f58d6e4 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -139,7 +139,7 @@ public function hasTable(string $tableName): bool $parts = $this->getSchemaName($tableName); /** @var array $result */ $result = $this->query( - "SELECT count(*) as [count] FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ?", + 'SELECT count(*) as [count] FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ?', [$parts['schema'], $parts['table']] )->fetch('assoc'); @@ -351,7 +351,7 @@ public function getColumnComment(string $tableName, ?string $columnName): ?strin AND columns.column_id = extended_properties.minor_id AND extended_properties.name = 'MS_Description' WHERE schemas.[name] = ? AND tables.[name] = ? AND columns.[name] = ?"; - $params = [ $this->schema, $tableName, (string)$columnName]; + $params = [$this->schema, $tableName, (string)$columnName]; $row = $this->query($sql, $params)->fetch('assoc'); if ($row) { @@ -1095,7 +1095,7 @@ public function hasDatabase(string $name): bool { /** @var array $result */ $result = $this->query( - "SELECT count(*) as [count] FROM master.dbo.sysdatabases WHERE [name] = ?", + 'SELECT count(*) as [count] FROM master.dbo.sysdatabases WHERE [name] = ?', [$name] )->fetch('assoc'); @@ -1257,13 +1257,8 @@ public function createSchema(string $schemaName = 'public'): void */ public function hasSchema(string $schemaName): bool { - $sql = sprintf( - 'SELECT count(*) AS [count] - FROM sys.schemas - WHERE name = %s', - $this->quoteString($schemaName) - ); - $result = $this->fetchRow($sql); + $sql = 'SELECT count(*) AS [count] FROM sys.schemas WHERE name = ?'; + $result = $this->query($sql, [$schemaName])->fetch('assoc'); if (!$result) { return false; } From cfa96ea8535c696f3ec4cdd1db8ee7cc7b161460 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 23:05:07 -0500 Subject: [PATCH 06/12] Fix errors --- src/Db/Adapter/MysqlAdapter.php | 4 ++-- src/Db/Adapter/SqliteAdapter.php | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 6b6051a8..22c9c2ca 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -830,8 +830,8 @@ protected function getForeignKeys(string $tableName): array REFERENCED_COLUMN_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE REFERENCED_TABLE_NAME IS NOT NULL - AND TABLE_SCHEMA = %s - AND TABLE_NAME = '%s' + AND TABLE_SCHEMA = ? + AND TABLE_NAME = ? ORDER BY POSITION_IN_UNIQUE_CONSTRAINT", $params )->fetchAll('assoc'); diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index 58020966..dbc60c39 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -326,12 +326,15 @@ protected function resolveTable(string $tableName): array } else { $master = sprintf('%s.%s', $this->quoteColumnName($schema), 'sqlite_master'); } + $rows = []; try { - $params = [$table]; - $rows = $this->query( + $result = $this->query( "SELECT name FROM {$master} WHERE type = 'table' AND lower(name) = ?", - $params - )->fetchAll('assoc'); + [$table] + ); + if ($result) { + $rows = $result->fetchAll('assoc'); + } } catch (PDOException $e) { // an exception can occur if the schema part of the table refers to a database which is not attached break; From 8fe2b0bd74be70af693fde1122f6d41f01880768 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 23:05:59 -0500 Subject: [PATCH 07/12] Fix another error --- src/Db/Adapter/SqlserverAdapter.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index 2f58d6e4..b93a97e9 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -375,8 +375,7 @@ public function getColumns(string $tableName): array NUMERIC_SCALE AS [scale], ORDINAL_POSITION AS [ordinal_position], COLUMNPROPERTY(object_id(TABLE_NAME), COLUMN_NAME, 'IsIdentity') as [identity] FROM INFORMATION_SCHEMA.COLUMNS - WHERE TABLE_SCHEMA = ? - WHERE TABLE_NAME = ? + WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ? ORDER BY ordinal_position"; $rows = $this->query($sql, [$parts['schema'], $parts['table']]) ->fetchAll('assoc'); From 195cbb4ce8b645a065984274120b30179559c777 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 23:08:32 -0500 Subject: [PATCH 08/12] phpstan doesn't like this --- src/Db/Adapter/SqliteAdapter.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index dbc60c39..cb43cc38 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -332,9 +332,7 @@ protected function resolveTable(string $tableName): array "SELECT name FROM {$master} WHERE type = 'table' AND lower(name) = ?", [$table] ); - if ($result) { - $rows = $result->fetchAll('assoc'); - } + $rows = $result->fetchAll('assoc'); } catch (PDOException $e) { // an exception can occur if the schema part of the table refers to a database which is not attached break; From 7dd5472e1e346a48b3d3cbcab58d4094963f00e5 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 23:19:38 -0500 Subject: [PATCH 09/12] Another mysql error --- src/Db/Adapter/MysqlAdapter.php | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index 22c9c2ca..f290faa1 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -812,29 +812,31 @@ public function hasForeignKey(string $tableName, $columns, ?string $constraint = */ protected function getForeignKeys(string $tableName): array { + $schema = null; if (strpos($tableName, '.') !== false) { [$schema, $tableName] = explode('.', $tableName); } - $foreignKeys = []; - $params = [ - empty($schema) ? 'DATABASE()' : "'$schema'", - $tableName, - ]; - $rows = $this->query( - "SELECT + $params = []; + $query = "SELECT CONSTRAINT_NAME, CONCAT(TABLE_SCHEMA, '.', TABLE_NAME) AS TABLE_NAME, COLUMN_NAME, CONCAT(REFERENCED_TABLE_SCHEMA, '.', REFERENCED_TABLE_NAME) AS REFERENCED_TABLE_NAME, REFERENCED_COLUMN_NAME FROM information_schema.KEY_COLUMN_USAGE - WHERE REFERENCED_TABLE_NAME IS NOT NULL - AND TABLE_SCHEMA = ? - AND TABLE_NAME = ? - ORDER BY POSITION_IN_UNIQUE_CONSTRAINT", - $params - )->fetchAll('assoc'); + WHERE REFERENCED_TABLE_NAME IS NOT NULL"; + + if ($schema) { + $query .= ' AND TABLE_SCHEMA = ?'; + $params[] = $schema; + } + + $query .= ' AND TABLE_NAME = ? ORDER BY POSITION_IN_UNIQUE_CONSTRAINT'; + $params[] = $tableName; + + $foreignKeys = []; + $rows = $this->query($query, $params)->fetchAll('assoc'); foreach ($rows as $row) { $foreignKeys[$row['CONSTRAINT_NAME']]['table'] = $row['TABLE_NAME']; $foreignKeys[$row['CONSTRAINT_NAME']]['columns'][] = $row['COLUMN_NAME']; From 487d88e6ae60b505a8f6d8f6e150d131ddf472ed Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 23:42:53 -0500 Subject: [PATCH 10/12] Restore fix and suppress phpstan --- phpstan-baseline.neon | 6 ++++++ src/Db/Adapter/SqliteAdapter.php | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 362adcc8..ed5f2611 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -96,6 +96,12 @@ parameters: count: 1 path: src/Db/Adapter/SqliteAdapter.php + - + message: '#^Strict comparison using \!\=\= between Cake\\Database\\StatementInterface and null will always evaluate to true\.$#' + identifier: notIdentical.alwaysTrue + count: 1 + path: src/Db/Adapter/SqliteAdapter.php + - message: '#^PHPDoc tag @return with type Phinx\\Db\\Adapter\\AdapterInterface is not subtype of native type Migrations\\Db\\Adapter\\AdapterInterface\.$#' identifier: return.phpDocType diff --git a/src/Db/Adapter/SqliteAdapter.php b/src/Db/Adapter/SqliteAdapter.php index cb43cc38..09fa5c24 100644 --- a/src/Db/Adapter/SqliteAdapter.php +++ b/src/Db/Adapter/SqliteAdapter.php @@ -332,7 +332,10 @@ protected function resolveTable(string $tableName): array "SELECT name FROM {$master} WHERE type = 'table' AND lower(name) = ?", [$table] ); - $rows = $result->fetchAll('assoc'); + // null on error + if ($result !== null) { + $rows = $result->fetchAll('assoc'); + } } catch (PDOException $e) { // an exception can occur if the schema part of the table refers to a database which is not attached break; From 1a7cd2760c6423c194079c927caa88b993acadf5 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 15 Dec 2024 23:58:23 -0500 Subject: [PATCH 11/12] Fix a missing condition --- src/Db/Adapter/MysqlAdapter.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index f290faa1..69618b57 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -830,6 +830,8 @@ protected function getForeignKeys(string $tableName): array if ($schema) { $query .= ' AND TABLE_SCHEMA = ?'; $params[] = $schema; + } else { + $query .= ' AND TABLE_SCHEMA = DATABASE()'; } $query .= ' AND TABLE_NAME = ? ORDER BY POSITION_IN_UNIQUE_CONSTRAINT'; From dbb163c9fa1c7f5d882c37c67bb9665f24011fe2 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 16 Dec 2024 18:03:12 -0500 Subject: [PATCH 12/12] Fix mistake --- src/Db/Adapter/SqlserverAdapter.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index b93a97e9..77a77945 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -816,8 +816,8 @@ public function getPrimaryKey(string $tableName): array JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS kcu ON tc.CONSTRAINT_NAME = kcu.CONSTRAINT_NAME WHERE CONSTRAINT_TYPE = 'PRIMARY KEY' - AND tc.CONSTRAINT_SCHEMA = '%s' - AND tc.TABLE_NAME = '%s' + AND tc.CONSTRAINT_SCHEMA = ? + AND tc.TABLE_NAME = ? ORDER BY kcu.ORDINAL_POSITION", [$parts['schema'], $parts['table']] )->fetchAll('assoc');