From f5bf5aedc1c6ef035d4f5c5e14c896e59bb8efab Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Fri, 3 Feb 2023 21:02:40 +0000 Subject: [PATCH] Modernize pgsql driver for PHP 8.1 --- phpcs.xml.dist | 5 --- phpstan.neon.dist | 13 ------ psalm.xml.dist | 31 ++------------ src/Driver/PgSQL/Connection.php | 21 ++-------- src/Driver/PgSQL/Driver.php | 4 +- src/Driver/PgSQL/Exception.php | 3 +- src/Driver/PgSQL/Result.php | 74 +++++++-------------------------- src/Driver/PgSQL/Statement.php | 38 ++++++----------- 8 files changed, 37 insertions(+), 152 deletions(-) diff --git a/phpcs.xml.dist b/phpcs.xml.dist index a1a58d354e0..6408fc97ede 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -53,11 +53,6 @@ */tests/* - - - src/Driver/PgSQL/Result.php - - tests/* diff --git a/phpstan.neon.dist b/phpstan.neon.dist index fb2ae351bcc..dcdddbb61a5 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -91,23 +91,10 @@ parameters: # We don't need to declare the return type *that* fine-grained. - '~^Method Doctrine\\DBAL\\Driver\\PDO\\Statement\:\:convertParamType\(\) never returns \d+ so it can be removed from the return type\.$~' - # PgSql objects are represented as resources in PHP 7.4 - - '~ expects PgSql\\Connection(:?\|(?:string|null))?, PgSql\\Connection\|resource given\.$~' - - '~ expects PgSql\\Result, PgSql\\Result\|resource given\.$~' - # PHPStan does not understand the array shapes returned by pg_fetch_*() methods. - '~^Parameter #1 \$row of method Doctrine\\DBAL\\Driver\\PgSQL\\Result\:\:mapAssociativeRow\(\) expects array, array given\.$~' - '~^Parameter #1 \$row of method Doctrine\\DBAL\\Driver\\PgSQL\\Result\:\:mapNumericRow\(\) expects array, array given\.$~' - # Ignore isset() checks in destructors. - - '~^Property Doctrine\\DBAL\\Driver\\PgSQL\\Connection\:\:\$connection \(PgSql\\Connection\|resource\) in isset\(\) is not nullable\.$~' - - '~^Property Doctrine\\DBAL\\Driver\\PgSQL\\Statement\:\:\$connection \(PgSql\\Connection\|resource\) in isset\(\) is not nullable\.$~' - - # On PHP 7.4, pg_fetch_all() might return false for empty result sets. - - - message: '~^Strict comparison using === between array and false will always evaluate to false\.$~' - paths: - - src/Driver/PgSQL/Result.php includes: - vendor/phpstan/phpstan-phpunit/extension.neon - vendor/phpstan/phpstan-phpunit/rules.neon diff --git a/psalm.xml.dist b/psalm.xml.dist index 695c8ceaabe..5fffb030256 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -118,32 +118,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -227,8 +201,9 @@ - - + + + diff --git a/src/Driver/PgSQL/Connection.php b/src/Driver/PgSQL/Connection.php index b46eab3bd59..c7c6e575493 100644 --- a/src/Driver/PgSQL/Connection.php +++ b/src/Driver/PgSQL/Connection.php @@ -8,12 +8,8 @@ use Doctrine\DBAL\Driver\Exception\NoIdentityValue; use Doctrine\DBAL\SQL\Parser; use PgSql\Connection as PgSqlConnection; -use TypeError; use function assert; -use function gettype; -use function is_object; -use function is_resource; use function pg_close; use function pg_escape_literal; use function pg_get_result; @@ -22,24 +18,14 @@ use function pg_send_prepare; use function pg_send_query; use function pg_version; -use function sprintf; use function uniqid; final class Connection implements ConnectionInterface { - private Parser $parser; + private readonly Parser $parser; - /** @param PgSqlConnection|resource $connection */ - public function __construct(private mixed $connection) + public function __construct(private readonly PgSqlConnection $connection) { - if (! is_resource($connection) && ! $connection instanceof PgSqlConnection) { - throw new TypeError(sprintf( - 'Expected connection to be a resource or an instance of %s, got %s.', - PgSqlConnection::class, - is_object($connection) ? $connection::class : gettype($connection), - )); - } - $this->parser = new Parser(false); } @@ -136,8 +122,7 @@ public function getServerVersion(): string return (string) pg_version($this->connection)['server']; } - /** @return PgSqlConnection|resource */ - public function getNativeConnection() + public function getNativeConnection(): PgSqlConnection { return $this->connection; } diff --git a/src/Driver/PgSQL/Driver.php b/src/Driver/PgSQL/Driver.php index 664a633a5e7..fc927e2d20d 100644 --- a/src/Driver/PgSQL/Driver.php +++ b/src/Driver/PgSQL/Driver.php @@ -75,11 +75,11 @@ private function constructConnectionString( 'password' => $params['password'] ?? null, 'sslmode' => $params['sslmode'] ?? null, ], - static fn ($value) => $value !== '' && $value !== null, + static fn (int|string|null $value) => $value !== '' && $value !== null, ); return implode(' ', array_map( - static fn ($value, string $key) => sprintf("%s='%s'", $key, addslashes($value)), + static fn (int|string $value, string $key) => sprintf("%s='%s'", $key, addslashes((string) $value)), array_values($components), array_keys($components), )); diff --git a/src/Driver/PgSQL/Exception.php b/src/Driver/PgSQL/Exception.php index d7fab7e4960..3036e556dda 100644 --- a/src/Driver/PgSQL/Exception.php +++ b/src/Driver/PgSQL/Exception.php @@ -19,8 +19,7 @@ */ final class Exception extends AbstractException { - /** @param PgSqlResult|resource $result */ - public static function fromResult($result): self + public static function fromResult(PgSqlResult $result): self { $sqlstate = pg_result_error_field($result, PGSQL_DIAG_SQLSTATE); if ($sqlstate === false) { diff --git a/src/Driver/PgSQL/Result.php b/src/Driver/PgSQL/Result.php index e893fb200d4..9838f7fc9b2 100644 --- a/src/Driver/PgSQL/Result.php +++ b/src/Driver/PgSQL/Result.php @@ -8,15 +8,11 @@ use Doctrine\DBAL\Driver\PgSQL\Exception\UnexpectedValue; use Doctrine\DBAL\Driver\Result as ResultInterface; use PgSql\Result as PgSqlResult; -use TypeError; use function array_keys; use function array_map; use function assert; -use function gettype; use function hex2bin; -use function is_object; -use function is_resource; use function pg_affected_rows; use function pg_fetch_all; use function pg_fetch_all_columns; @@ -26,7 +22,6 @@ use function pg_field_type; use function pg_free_result; use function pg_num_fields; -use function sprintf; use function substr; use const PGSQL_ASSOC; @@ -35,20 +30,10 @@ final class Result implements ResultInterface { - /** @var PgSqlResult|resource|null */ - private mixed $result; + private ?PgSqlResult $result; - /** @param PgSqlResult|resource $result */ - public function __construct(mixed $result) + public function __construct(PgSqlResult $result) { - if (! is_resource($result) && ! $result instanceof PgSqlResult) { - throw new TypeError(sprintf( - 'Expected result to be a resource or an instance of %s, got %s.', - PgSqlResult::class, - is_object($result) ? $result::class : gettype($result), - )); - } - $this->result = $result; } @@ -95,17 +80,11 @@ public function fetchAllNumeric(): array return []; } - $resultSet = pg_fetch_all($this->result, PGSQL_NUM); - // On PHP 7.4, pg_fetch_all() might return false for empty result sets. - if ($resultSet === false) { - return []; - } - $types = $this->fetchNumericColumnTypes(); return array_map( fn (array $row) => $this->mapNumericRow($row, $types), - $resultSet, + pg_fetch_all($this->result, PGSQL_NUM), ); } @@ -116,17 +95,11 @@ public function fetchAllAssociative(): array return []; } - $resultSet = pg_fetch_all($this->result, PGSQL_ASSOC); - // On PHP 7.4, pg_fetch_all() might return false for empty result sets. - if ($resultSet === false) { - return []; - } - $types = $this->fetchAssociativeColumnTypes(); return array_map( fn (array $row) => $this->mapAssociativeRow($row, $types), - $resultSet, + pg_fetch_all($this->result, PGSQL_ASSOC), ); } @@ -242,32 +215,17 @@ private function mapType(string $postgresType, ?string $value): string|int|float return null; } - switch ($postgresType) { - case 'bool': - switch ($value) { - case 't': - return true; - case 'f': - return false; - } - - throw UnexpectedValue::new($value, $postgresType); - - case 'bytea': - return hex2bin(substr($value, 2)); - - case 'float4': - case 'float8': - return (float) $value; - - case 'int2': - case 'int4': - return (int) $value; - - case 'int8': - return PHP_INT_SIZE >= 8 ? (int) $value : $value; - } - - return $value; + return match ($postgresType) { + 'bool' => match ($value) { + 't' => true, + 'f' => false, + default => throw UnexpectedValue::new($value, $postgresType), + }, + 'bytea' => hex2bin(substr($value, 2)), + 'float4', 'float8' => (float) $value, + 'int2', 'int4' => (int) $value, + 'int8' => PHP_INT_SIZE >= 8 ? (int) $value : $value, + default => $value, + }; } } diff --git a/src/Driver/PgSQL/Statement.php b/src/Driver/PgSQL/Statement.php index 19e0dde4bad..ee28d17223e 100644 --- a/src/Driver/PgSQL/Statement.php +++ b/src/Driver/PgSQL/Statement.php @@ -8,12 +8,8 @@ use Doctrine\DBAL\Driver\Statement as StatementInterface; use Doctrine\DBAL\ParameterType; use PgSql\Connection as PgSqlConnection; -use TypeError; use function assert; -use function gettype; -use function is_object; -use function is_resource; use function ksort; use function pg_escape_bytea; use function pg_escape_identifier; @@ -22,7 +18,6 @@ use function pg_query; use function pg_result_error; use function pg_send_execute; -use function sprintf; final class Statement implements StatementInterface { @@ -32,19 +27,12 @@ final class Statement implements StatementInterface /** @psalm-var array */ private array $parameterTypes = []; - /** - * @param PgSqlConnection|resource $connection - * @param array $parameterMap - */ - public function __construct(private mixed $connection, private string $name, private array $parameterMap) - { - if (! is_resource($connection) && ! $connection instanceof PgSqlConnection) { - throw new TypeError(sprintf( - 'Expected connection to be a resource or an instance of %s, got %s.', - PgSqlConnection::class, - is_object($connection) ? $connection::class : gettype($connection), - )); - } + /** @param array $parameterMap */ + public function __construct( + private readonly PgSqlConnection $connection, + private readonly string $name, + private readonly array $parameterMap, + ) { } public function __destruct() @@ -77,14 +65,12 @@ public function execute(): Result $escapedParameters = []; foreach ($this->parameters as $parameter => $value) { - switch ($this->parameterTypes[$parameter]) { - case ParameterType::BINARY: - case ParameterType::LARGE_OBJECT: - $escapedParameters[] = $value === null ? null : pg_escape_bytea($this->connection, $value); - break; - default: - $escapedParameters[] = $value; - } + $escapedParameters[] = match ($this->parameterTypes[$parameter]) { + ParameterType::BINARY, ParameterType::LARGE_OBJECT => $value === null + ? null + : pg_escape_bytea($this->connection, $value), + default => $value, + }; } if (@pg_send_execute($this->connection, $this->name, $escapedParameters) !== true) {