Skip to content

Deallocate prepared statements #6665

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

Merged
merged 20 commits into from
Oct 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d0df1e2
feat: Abstract method and database dependants versions of the "close"…
fpoy Oct 10, 2022
7e244b3
test: add test for deallocation of prepared query
fpoy Oct 11, 2022
ce3b698
docs: remove todo comment
fpoy Oct 11, 2022
63d50b4
fix: escape identifier for Postgre's DEALLOCATE statement
fpoy Oct 11, 2022
344bc6f
fix: Undefined property: CodeIgniter\Database\OCI8\PreparedQuery::$st…
fpoy Oct 11, 2022
7a4017f
fix: Removes recasting of the same type
fpoy Oct 11, 2022
1523d83
docs: add breaking change to the changelog
fpoy Oct 12, 2022
a4f09b1
docs: add enhancement the changelog
fpoy Oct 12, 2022
072a620
docs: add inline markup for close() method
fpoy Oct 13, 2022
78ae90d
fix: change the visibility of close() to "protected"
fpoy Oct 13, 2022
0b6e0e4
refactor: move duplication in the implementations of _close() to the …
fpoy Oct 13, 2022
5f482d1
typo: database-dependent
fpoy Oct 13, 2022
82e37fd
refactor: if pg_query() returns an object, _close returns true
fpoy Oct 13, 2022
3b7465f
docs: complete the example query with the error return
fpoy Oct 13, 2022
77377a7
fix: return BadMethodCallException when trying to close a non-existin…
fpoy Oct 14, 2022
78a34c9
tests: use expectException() for unit-tests, not try/catch
fpoy Oct 14, 2022
dbe460e
fix: do not unset statement after closing it, instead set to null
fpoy Oct 14, 2022
f859e46
docs: add @throws statement in the docblock of close()
fpoy Oct 14, 2022
2f5bce7
fix phpstan after rebase
fpoy Oct 17, 2022
1a72ba8
test: remove spurious null check in unit test
fpoy Oct 17, 2022
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
14 changes: 2 additions & 12 deletions phpstan-baseline.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
23 changes: 17 additions & 6 deletions system/Database/BasePreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ abstract class BasePreparedQuery implements PreparedQueryInterface
/**
* The prepared statement itself.
*
* @var object|resource
* @var object|resource|null
*/
protected $statement;

Expand Down Expand Up @@ -147,17 +147,28 @@ abstract public function _execute(array $data): bool;
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.
*/
Expand Down
8 changes: 8 additions & 0 deletions system/Database/MySQLi/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,12 @@ public function _getResult()
{
return $this->statement->get_result();
}

/**
* Deallocate prepared statements
*/
protected function _close(): bool
{
return $this->statement->close();
}
}
10 changes: 9 additions & 1 deletion system/Database/OCI8/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function _prepare(string $sql, array $options = [])
*/
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.');
}

Expand Down Expand Up @@ -98,6 +98,14 @@ public function _getResult()
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.
Expand Down
8 changes: 8 additions & 0 deletions system/Database/Postgre/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ public function _getResult()
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.
Expand Down
8 changes: 8 additions & 0 deletions system/Database/SQLSRV/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,14 @@ public function _getResult()
return $this->result;
}

/**
* Deallocate prepared statements
*/
protected function _close(): bool
{
return sqlsrv_free_stmt($this->statement);
}

/**
* Handle parameters
*/
Expand Down
10 changes: 8 additions & 2 deletions system/Database/SQLite3/PreparedQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ public function _prepare(string $sql, array $options = [])
/**
* 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
{
Expand Down Expand Up @@ -93,4 +91,12 @@ public function _getResult()
{
return $this->result;
}

/**
* Deallocate prepared statements
*/
protected function _close(): bool
{
return $this->statement->close();
}
}
39 changes: 38 additions & 1 deletion tests/system/Database/Live/PreparedQueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CodeIgniter\Database\Live;

use BadMethodCallException;
use CodeIgniter\Database\BasePreparedQuery;
use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\Query;
Expand Down Expand Up @@ -41,8 +42,10 @@ protected function tearDown(): void
{
parent::tearDown();

if ($this->query !== null) {
try {
$this->query->close();
} catch (BadMethodCallException $e) {
$this->query = null;
}
}

Expand Down Expand Up @@ -148,4 +151,38 @@ public function testExecuteRunsInvalidQuery()

$this->query->execute('foo', '[email protected]', 'US');
}

public function testDeallocatePreparedQueryThenTryToExecute()
{
$this->query = $this->db->prepare(static fn ($db) => $db->table('user')->insert([
'name' => 'a',
'email' => '[email protected]',
'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', '[email protected]', 'GB');
}

public function testDeallocatePreparedQueryThenTryToClose()
{
$this->query = $this->db->prepare(static fn ($db) => $db->table('user')->insert([
'name' => 'a',
'email' => '[email protected]',
'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();
}
}
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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``.
Expand Down Expand Up @@ -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
=====
Expand Down
4 changes: 4 additions & 0 deletions user_guide_src/source/database/queries.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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()
-------

Expand All @@ -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()
----------------

Expand Down
6 changes: 5 additions & 1 deletion user_guide_src/source/database/queries/020.php
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
<?php

$pQuery->close();
if ($pQuery->close()) {
echo 'Success!';
} else {
echo 'Deallocation of prepared statements failed!';
}