Merge pull request #6665 from fpoy/FEAT_deallocate_prepared_statements

Deallocate prepared statements

phpstan-baseline.neon.dist

@@ -216,7 +216,7 @@ parameters:
 			path: system/Database/MySQLi/PreparedQuery.php
 
 		-
-			message: "#^Property CodeIgniter\\\\Database\\\\BasePreparedQuery\\:\\:\\$statement \\(object\\|resource\\) in isset\\(\\) is not nullable\\.$#"
+			message: "#^Cannot call method close\\(\\) on object\\|resource\\.$#"
 			count: 1
 			path: system/Database/MySQLi/PreparedQuery.php
 
@@ -265,11 +265,6 @@ parameters:
 			count: 1
 			path: system/Database/Postgre/Connection.php
 
-		-
-			message: "#^Property CodeIgniter\\\\Database\\\\BasePreparedQuery\\:\\:\\$statement \\(object\\|resource\\) in isset\\(\\) is not nullable\\.$#"
-			count: 1
-			path: system/Database/Postgre/PreparedQuery.php
-
 		-
 			message: "#^Access to an undefined property CodeIgniter\\\\Database\\\\BaseConnection\\:\\:\\$schema\\.$#"
 			count: 2
@@ -280,11 +275,6 @@ parameters:
 			count: 13
 			path: system/Database/SQLSRV/Forge.php
 
-		-
-			message: "#^Property CodeIgniter\\\\Database\\\\BasePreparedQuery\\:\\:\\$statement \\(object\\|resource\\) in isset\\(\\) is not nullable\\.$#"
-			count: 1
-			path: system/Database/SQLSRV/PreparedQuery.php
-
 		-
 			message: "#^Cannot call method changes\\(\\) on bool\\|object\\|resource\\.$#"
 			count: 1
@@ -351,7 +341,7 @@ parameters:
 			path: system/Database/SQLite3/PreparedQuery.php
 
 		-
-			message: "#^Property CodeIgniter\\\\Database\\\\BasePreparedQuery\\:\\:\\$statement \\(object\\|resource\\) in isset\\(\\) is not nullable\\.$#"
+			message: "#^Cannot call method close\\(\\) on object\\|resource\\.$#"
 			count: 1
 			path: system/Database/SQLite3/PreparedQuery.php
 

system/Database/BasePreparedQuery.php

@@ -25,7 +25,7 @@ abstract class BasePreparedQuery implements PreparedQueryInterface
     /**
      * The prepared statement itself.
      *
-     * @var object|resource
+     * @var object|resource|null
      */
     protected $statement;
 
@@ -147,17 +147,28 @@ abstract class BasePreparedQuery implements PreparedQueryInterface
     abstract public function _getResult();
 
     /**
-     * Explicitly closes the statement.
+     * Explicitly closes the prepared statement.
+     *
+     * @throws BadMethodCallException
      */
-    public function close()
+    public function close(): bool
     {
-        if (! is_object($this->statement) || ! method_exists($this->statement, 'close')) {
-            return;
+        if (! isset($this->statement)) {
+            throw new BadMethodCallException('Cannot call close on a non-existing prepared statement.');
         }
 
-        $this->statement->close();
+        try {
+            return $this->_close();
+        } finally {
+            $this->statement = null;
+        }
     }
 
+    /**
+     * The database-dependent version of the close method.
+     */
+    abstract protected function _close(): bool;
+
     /**
      * Returns the SQL that has been prepared.
      */

system/Database/MySQLi/PreparedQuery.php

@@ -89,4 +89,12 @@ class PreparedQuery extends BasePreparedQuery
     {
         return $this->statement->get_result();
     }
+
+    /**
+     * Deallocate prepared statements
+     */
+    protected function _close(): bool
+    {
+        return $this->statement->close();
+    }
 }

system/Database/OCI8/PreparedQuery.php

@@ -68,7 +68,7 @@ class PreparedQuery extends BasePreparedQuery implements PreparedQueryInterface
      */
     public function _execute(array $data): bool
     {
-        if (null === $this->statement) {
+        if (! isset($this->statement)) {
             throw new BadMethodCallException('You must call prepare before trying to execute a prepared statement.');
         }
 
@@ -98,6 +98,14 @@ class PreparedQuery extends BasePreparedQuery implements PreparedQueryInterface
         return $this->statement;
     }
 
+    /**
+     * Deallocate prepared statements
+     */
+    protected function _close(): bool
+    {
+        return oci_free_statement($this->statement);
+    }
+
     /**
      * Replaces the ? placeholders with :0, :1, etc parameters for use
      * within the prepared query.

system/Database/Postgre/PreparedQuery.php

@@ -98,6 +98,14 @@ class PreparedQuery extends BasePreparedQuery
         return $this->result;
     }
 
+    /**
+     * Deallocate prepared statements
+     */
+    protected function _close(): bool
+    {
+        return pg_query($this->db->connID, 'DEALLOCATE "' . $this->db->escapeIdentifiers($this->name) . '"') !== false;
+    }
+
     /**
      * Replaces the ? placeholders with $1, $2, etc parameters for use
      * within the prepared query.

system/Database/SQLSRV/PreparedQuery.php

@@ -116,6 +116,14 @@ class PreparedQuery extends BasePreparedQuery
         return $this->result;
     }
 
+    /**
+     * Deallocate prepared statements
+     */
+    protected function _close(): bool
+    {
+        return sqlsrv_free_stmt($this->statement);
+    }
+
     /**
      * Handle parameters
      */

system/Database/SQLite3/PreparedQuery.php

@@ -56,8 +56,6 @@ class PreparedQuery extends BasePreparedQuery
     /**
      * Takes a new set of data and runs it against the currently
      * prepared query. Upon success, will return a Results object.
-     *
-     * @todo finalize()
      */
     public function _execute(array $data): bool
     {
@@ -93,4 +91,12 @@ class PreparedQuery extends BasePreparedQuery
     {
         return $this->result;
     }
+
+    /**
+     * Deallocate prepared statements
+     */
+    protected function _close(): bool
+    {
+        return $this->statement->close();
+    }
 }

tests/system/Database/Live/PreparedQueryTest.php

@@ -11,6 +11,7 @@
 
 namespace CodeIgniter\Database\Live;
 
+use BadMethodCallException;
 use CodeIgniter\Database\BasePreparedQuery;
 use CodeIgniter\Database\Exceptions\DatabaseException;
 use CodeIgniter\Database\Query;
@@ -41,8 +42,10 @@ final class PreparedQueryTest extends CIUnitTestCase
     {
         parent::tearDown();
 
-        if ($this->query !== null) {
+        try {
             $this->query->close();
+        } catch (BadMethodCallException $e) {
+            $this->query = null;
         }
     }
 
@@ -148,4 +151,38 @@ final class PreparedQueryTest extends CIUnitTestCase
 
         $this->query->execute('foo', 'foo@example.com', 'US');
     }
+
+    public function testDeallocatePreparedQueryThenTryToExecute()
+    {
+        $this->query = $this->db->prepare(static fn ($db) => $db->table('user')->insert([
+            'name'    => 'a',
+            'email'   => 'b@example.com',
+            'country' => 'x',
+        ]));
+
+        $this->query->close();
+
+        // Try to execute a non-existing prepared statement
+        $this->expectException(BadMethodCallException::class);
+        $this->expectExceptionMessage('You must call prepare before trying to execute a prepared statement.');
+
+        $this->query->execute('bar', 'bar@example.com', 'GB');
+    }
+
+    public function testDeallocatePreparedQueryThenTryToClose()
+    {
+        $this->query = $this->db->prepare(static fn ($db) => $db->table('user')->insert([
+            'name'    => 'a',
+            'email'   => 'b@example.com',
+            'country' => 'x',
+        ]));
+
+        $this->query->close();
+
+        // Try to close a non-existing prepared statement
+        $this->expectException(BadMethodCallException::class);
+        $this->expectExceptionMessage('Cannot call close on a non-existing prepared statement.');
+
+        $this->query->close();
+    }
 }

user_guide_src/source/changelogs/v4.3.0.rst

@@ -90,6 +90,7 @@ The return value of  ``Validation::loadRuleGroup()`` has been changed ``null`` t
 Others
 ------
 
+- The return type of ``CodeIgniter\Database\BasePreparedQuery::close()`` has been changed to ``bool``.
 - The return type of ``CodeIgniter\Database\Database::loadForge()`` has been changed to ``Forge``.
 - The return type of ``CodeIgniter\Database\Database::loadUtils()`` has been changed to ``BaseUtils``.
 - Parameter ``$column`` has changed in ``Table::dropForeignKey()`` to ``$foreignName``.
@@ -142,6 +143,7 @@ Database
 - Added the ability to manually set index names. These methods include: ``Forge::addKey()``, ``Forge::addPrimaryKey()``, and ``Forge::addUniqueKey()``
 - Fixed ``Forge::dropKey()`` to allow droping unique indexes. This required the ``DROP CONSTRAINT`` SQL command.
 - Added ``upsert()`` and ``upsertBatch()`` methods to QueryBuilder. See :ref:`upsert-data`.
+- ``BasePreparedQuery::close()`` now deallocates the prepared statement in all DBMS. Previously, they were not deallocated in Postgre, SQLSRV and OCI8. See :ref:`database-queries-stmt-close`.
 
 Model
 =====

user_guide_src/source/database/queries.rst

@@ -232,6 +232,8 @@ Other Methods
 
 In addition to these two primary methods, the prepared query object also has the following methods:
 
+.. _database-queries-stmt-close:
+
 close()
 -------
 
@@ -240,6 +242,8 @@ close out the prepared statement when you're done with it:
 
 .. literalinclude:: queries/020.php
 
+.. note:: Since v4.3.0, the ``close()`` method deallocates the prepared statement in all DBMS. Previously, they were not deallocated in Postgre, SQLSRV and OCI8.
+
 getQueryString()
 ----------------
 

user_guide_src/source/database/queries/020.php

@@ -1,3 +1,7 @@
 <?php
 
-$pQuery->close();
+if ($pQuery->close()) {
+    echo 'Success!';
+} else {
+    echo 'Deallocation of prepared statements failed!';
+}