Skip to content

Commit

Permalink
Less sprintf in adapters (#788)
Browse files Browse the repository at this point in the history 
* Improve test run without optional connections
* Start removing sprintf from PdoAdapter
* Use prepared statements in postgres adapter
* Remove more sprintf from adapters
* Fix more queries from 4.next
* Fix errors
* Fix another error
* phpstan doesn't like this
* Another mysql error
* Restore fix and suppress phpstan
* Fix a missing condition
* Fix mistake
  • Loading branch information
@markstory
markstory authored Dec 17, 2024
1 parent c88055d commit 2faa558
Show file tree
Hide file tree
Showing 9 changed files with 176 additions and 184 deletions.
6 changes: 6 additions & 0 deletions phpstan-baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 36 additions & 31 deletions src/Db/Adapter/MysqlAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' => [],
Expand Down Expand Up @@ -809,26 +812,33 @@ 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 = [];
$rows = $this->fetchAll(sprintf(
"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 = %s
AND TABLE_NAME = '%s'
ORDER BY POSITION_IN_UNIQUE_CONSTRAINT",
empty($schema) ? 'DATABASE()' : "'$schema'",
$tableName
));
WHERE REFERENCED_TABLE_NAME IS NOT NULL";

if ($schema) {
$query .= ' AND TABLE_SCHEMA = ?';
$params[] = $schema;
} else {
$query .= ' AND TABLE_SCHEMA = DATABASE()';
}

$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'];
Expand Down Expand Up @@ -1270,12 +1280,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)) {
Expand Down Expand Up @@ -1470,16 +1478,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 : [];
}
Expand Down
38 changes: 22 additions & 16 deletions src/Db/Adapter/PdoAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
54 changes: 27 additions & 27 deletions src/Db/Adapter/PostgresAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -833,20 +832,23 @@ 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
FROM information_schema.table_constraints AS tc
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' => [],
Expand Down Expand Up @@ -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,
Expand All @@ -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'];
Expand Down Expand Up @@ -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;
}
Expand Down
34 changes: 20 additions & 14 deletions src/Db/Adapter/SqliteAdapter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,16 @@ protected function resolveTable(string $tableName): array
} else {
$master = sprintf('%s.%s', $this->quoteColumnName($schema), 'sqlite_master');
}
$rows = [];
try {
$rows = $this->fetchAll(sprintf("SELECT name FROM %s WHERE type='table' AND lower(name) = %s", $master, $this->quoteString($table)));
$result = $this->query(
"SELECT name FROM {$master} WHERE type = 'table' AND lower(name) = ?",
[$table]
);
// 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;
Expand Down Expand Up @@ -797,19 +805,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'];

Expand Down
Loading

0 comments on commit 2faa558

Please sign in to comment.