Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modernize pgsql driver for PHP 8.1 #5904

Merged
merged 1 commit into from
Feb 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@
<exclude-pattern>*/tests/*</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.Classes.RequireConstructorPropertyPromotion.RequiredConstructorPropertyPromotion">
<!-- CPP breaks static analysis here. -->
<exclude-pattern>src/Driver/PgSQL/Result.php</exclude-pattern>
</rule>

<rule ref="SlevomatCodingStandard.PHP.RequireExplicitAssertion.RequiredExplicitAssertion">
<exclude-pattern>tests/*</exclude-pattern>
</rule>
Expand Down
13 changes: 0 additions & 13 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string\|null>, array<int\|string, string\|null> given\.$~'
- '~^Parameter #1 \$row of method Doctrine\\DBAL\\Driver\\PgSQL\\Result\:\:mapNumericRow\(\) expects array<int, string\|null>, array<int\|string, string\|null> 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<int, 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
Expand Down
31 changes: 3 additions & 28 deletions psalm.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -118,32 +118,6 @@
<file name="src/Driver/AbstractSQLiteDriver.php"/>
</errorLevel>
</NullableReturnStatement>
<PossiblyInvalidArgument>
<errorLevel type="suppress">
<!-- PgSql objects are represented as resources in PHP 7.4 -->
<referencedFunction name="pg_affected_rows"/>
<referencedFunction name="pg_close"/>
<referencedFunction name="pg_escape_bytea"/>
<referencedFunction name="pg_escape_identifier"/>
<referencedFunction name="pg_escape_literal"/>
<referencedFunction name="pg_fetch_all"/>
<referencedFunction name="pg_fetch_all_columns"/>
<referencedFunction name="pg_fetch_assoc"/>
<referencedFunction name="pg_fetch_row"/>
<referencedFunction name="pg_field_name"/>
<referencedFunction name="pg_field_type"/>
<referencedFunction name="pg_free_result"/>
<referencedFunction name="pg_get_result"/>
<referencedFunction name="pg_last_error"/>
<referencedFunction name="pg_num_fields"/>
<referencedFunction name="pg_query"/>
<referencedFunction name="pg_result_error_field"/>
<referencedFunction name="pg_send_execute"/>
<referencedFunction name="pg_send_prepare"/>
<referencedFunction name="pg_send_query"/>
<referencedFunction name="pg_version"/>
</errorLevel>
</PossiblyInvalidArgument>
<PossiblyInvalidArrayOffset>
<errorLevel type="suppress">
<!-- $array[key($array)] is safe. -->
Expand Down Expand Up @@ -227,8 +201,9 @@
</TypeDoesNotContainNull>
<TypeDoesNotContainType>
<errorLevel type="suppress">
<!-- On PHP 7.4, pg_fetch_all() might return false for empty result sets. -->
<file name="src/Driver/PgSQL/Result.php"/>
<!-- Ignore isset() checks in destructors. -->
<file name="src/Driver/PgSQL/Connection.php"/>
<file name="src/Driver/PgSQL/Statement.php"/>
</errorLevel>
</TypeDoesNotContainType>
<UndefinedDocblockClass>
Expand Down
21 changes: 3 additions & 18 deletions src/Driver/PgSQL/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Driver/PgSQL/Driver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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),
));
Expand Down
3 changes: 1 addition & 2 deletions src/Driver/PgSQL/Exception.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
74 changes: 16 additions & 58 deletions src/Driver/PgSQL/Result.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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),
);
}

Expand All @@ -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),
);
}

Expand Down Expand Up @@ -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,
};
}
}
38 changes: 12 additions & 26 deletions src/Driver/PgSQL/Statement.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
{
Expand All @@ -32,19 +27,12 @@ final class Statement implements StatementInterface
/** @psalm-var array<int, ParameterType> */
private array $parameterTypes = [];

/**
* @param PgSqlConnection|resource $connection
* @param array<array-key, int> $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<array-key, int> $parameterMap */
public function __construct(
private readonly PgSqlConnection $connection,
private readonly string $name,
private readonly array $parameterMap,
) {
}

public function __destruct()
Expand Down Expand Up @@ -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) {
Expand Down