Merge pull request #5893 from derrabus/bugfix/deallocate-statements

Deallocate prepared statements in destructor

Author: derrabus

phpstan.neon.dist

@@ -148,6 +148,9 @@ parameters:
         - '~^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\\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\.$~'

psalm.xml.dist

@@ -646,6 +646,7 @@
                 <!-- PgSql objects are represented as resources in PHP 7.4 -->
                 <referencedFunction name="pg_affected_rows"/>
                 <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"/>
@@ -657,6 +658,7 @@
                 <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"/>
@@ -748,6 +750,9 @@
             <errorLevel type="suppress">
                 <!-- Running isset() checks on properties that should be initialized by setUp() is fine. -->
                 <directory name="tests"/>
+
+                <!-- Ignore isset() checks in destructors. -->
+                <file name="src/Driver/PgSQL/Statement.php"/>
             </errorLevel>
         </RedundantPropertyInitializationCheck>
         <ReferenceConstraintViolation>

src/Driver/PgSQL/Statement.php

@@ -18,8 +18,10 @@
 use function is_resource;
 use function ksort;
 use function pg_escape_bytea;
+use function pg_escape_identifier;
 use function pg_get_result;
 use function pg_last_error;
+use function pg_query;
 use function pg_result_error;
 use function pg_send_execute;
 use function sprintf;
@@ -59,6 +61,18 @@ public function __construct($connection, string $name, array $parameterMap)
         $this->parameterMap = $parameterMap;
     }
 
+    public function __destruct()
+    {
+        if (! isset($this->connection)) {
+            return;
+        }
+
+        @pg_query(
+            $this->connection,
+            'DEALLOCATE ' . pg_escape_identifier($this->connection, $this->name),
+        );
+    }
+
     /** {@inheritdoc} */
     public function bindValue($param, $value, $type = ParameterType::STRING): bool
     {

tests/Functional/Driver/PgSQL/StatementTest.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Tests\Functional\Driver\PgSQL;
+
+use Doctrine\DBAL\Driver\PgSQL\Statement;
+use Doctrine\DBAL\Exception\DriverException;
+use Doctrine\DBAL\Statement as WrapperStatement;
+use Doctrine\DBAL\Tests\FunctionalTestCase;
+use Doctrine\DBAL\Tests\TestUtil;
+use ReflectionProperty;
+
+use function sprintf;
+
+class StatementTest extends FunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        if (TestUtil::isDriverOneOf('pgsql')) {
+            return;
+        }
+
+        self::markTestSkipped('This test requires the pgsql driver.');
+    }
+
+    public function testStatementsAreDeallocatedProperly(): void
+    {
+        $statement = $this->connection->prepare('SELECT 1');
+
+        $property = new ReflectionProperty(WrapperStatement::class, 'stmt');
+        $property->setAccessible(true);
+
+        $driverStatement = $property->getValue($statement);
+
+        $property = new ReflectionProperty(Statement::class, 'name');
+        $property->setAccessible(true);
+
+        $name = $property->getValue($driverStatement);
+
+        unset($statement, $driverStatement);
+
+        $this->expectException(DriverException::class);
+        $this->expectExceptionMessageMatches('/prepared statement .* does not exist/');
+
+        $this->connection->executeQuery(sprintf('EXECUTE "%s"', $name));
+    }
+}