Issue #3137883 by mondrake, Pooja Ganjage, andypost, paulocs, anmolgoyal74, daffie, alexpott, catch: Deprecate passing a StatementInterface object to Connection::query

merge-requests/83/head
Alex Pott 2020-11-29 00:17:47 +00:00
parent b3d61bd368
commit 6aab22375b
No known key found for this signature in database
GPG Key ID: 31905460D4A69276
5 changed files with 76 additions and 28 deletions

View File

@ -749,11 +749,12 @@ abstract class Connection {
* statements.
*
* @param string|\Drupal\Core\Database\StatementInterface|\PDOStatement $query
* The query to execute. In most cases this will be a string containing
* an SQL query with placeholders. An already-prepared instance of
* StatementInterface may also be passed in order to allow calling
* code to manually bind variables to a query. If a
* StatementInterface is passed, the $args array will be ignored.
* The query to execute. This is a string containing an SQL query with
* placeholders.
* (deprecated) An already-prepared instance of StatementInterface or of
* \PDOStatement may also be passed in order to allow calling code to
* manually bind variables to a query. In such cases, the content of the
* $args array will be ignored.
* It is extremely rare that module code will need to pass a statement
* object to this method. It is used primarily for database drivers for
* databases that require special LOB field handling.
@ -792,14 +793,16 @@ abstract class Connection {
assert(!isset($options['target']), 'Passing "target" option to query() has no effect. See https://www.drupal.org/node/2993033');
try {
// We allow either a pre-bound statement object or a literal string.
// In either case, we want to end up with an executed statement object,
// which we pass to PDOStatement::execute.
// We allow either a pre-bound statement object (deprecated) or a literal
// string. In either case, we want to end up with an executed statement
// object, which we pass to PDOStatement::execute.
if ($query instanceof StatementInterface) {
@trigger_error('Passing a StatementInterface object as a $query argument to ' . __METHOD__ . ' is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Call the execute method from the StatementInterface object directly instead. See https://www.drupal.org/node/3154439', E_USER_DEPRECATED);
$stmt = $query;
$stmt->execute(NULL, $options);
}
elseif ($query instanceof \PDOStatement) {
@trigger_error('Passing a \\PDOStatement object as a $query argument to ' . __METHOD__ . ' is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Call the execute method from the StatementInterface object directly instead. See https://www.drupal.org/node/3154439', E_USER_DEPRECATED);
$stmt = $query;
$stmt->execute();
}
@ -878,6 +881,8 @@ abstract class Connection {
// Wrap the exception in another exception, because PHP does not allow
// overriding Exception::getMessage(). Its message is the extra database
// debug information.
// @todo in Drupal 10, remove checking if $query is a statement object.
// @see https://www.drupal.org/node/3154439
if ($query instanceof StatementInterface) {
$query_string = $query->getQueryString();
}

View File

@ -3,6 +3,8 @@
namespace Drupal\Core\Database\Driver\pgsql;
use Drupal\Core\Database\Database;
use Drupal\Core\Database\DatabaseExceptionWrapper;
use Drupal\Core\Database\IntegrityConstraintViolationException;
use Drupal\Core\Database\Query\Insert as QueryInsert;
// cSpell:ignore nextval setval
@ -84,18 +86,10 @@ class Insert extends QueryInsert {
}
}
// PostgreSQL requires the table name to be specified explicitly
// when requesting the last insert ID, so we pass that in via
// the options array.
$options = $this->queryOptions;
if (!empty($table_information->sequences)) {
$options['sequence_name'] = $table_information->sequences[0];
}
// If there are no sequences then we can't get a last insert id.
elseif ($options['return'] == Database::RETURN_INSERT_ID) {
$options['return'] = Database::RETURN_NULL;
}
// PostgreSQL requires the table name to be specified explicitly when
// requesting the last insert ID. If there are no sequences, then we can't
// get a last insert id.
$sequence_name = $table_information->sequences[0] ?? NULL;
// Create a savepoint so we can rollback a failed query. This is so we can
// mimic MySQL and SQLite transactions which don't fail if a single query
@ -103,15 +97,32 @@ class Insert extends QueryInsert {
// example, \Drupal\Core\Cache\DatabaseBackend.
$this->connection->addSavepoint();
try {
$stmt->execute(NULL, $this->queryOptions);
// Only use the returned last_insert_id if it is not already set.
if (!empty($last_insert_id)) {
$this->connection->query($stmt, [], $options);
}
else {
$last_insert_id = $this->connection->query($stmt, [], $options);
// PostgreSQL requires the table name to be specified explicitly when
// requesting the last insert ID.
if (!isset($last_insert_id)) {
if ($this->queryOptions['return'] === Database::RETURN_INSERT_ID && $sequence_name !== NULL) {
// cspell:ignore currval
$last_insert_id = $this->connection->query("SELECT currval('$sequence_name')")->fetchField();
}
else {
$last_insert_id = NULL;
}
}
$this->connection->releaseSavepoint();
}
catch (\PDOException $e) {
$this->connection->rollbackSavepoint();
$message = $e->getMessage() . ": " . $stmt->getQueryString();
// Match all SQLSTATE 23xxx errors.
if (substr($e->getCode(), -6, -3) == '23') {
throw new IntegrityConstraintViolationException($message, $e->getCode(), $e);
}
else {
throw new DatabaseExceptionWrapper($message, 0, $e->getCode());
}
}
catch (\Exception $e) {
$this->connection->rollbackSavepoint();
throw $e;

View File

@ -75,9 +75,10 @@ class Update extends QueryUpdate {
$this->connection->addSavepoint();
try {
$result = $this->connection->query($stmt, [], $options);
$stmt->execute(NULL, $options);
$this->connection->releaseSavepoint();
return $result;
$stmt->allowRowCount = TRUE;
return $stmt->rowCount();
}
catch (\Exception $e) {
$this->connection->rollbackSavepoint();

View File

@ -80,7 +80,7 @@ class Upsert extends QueryUpsert {
// example, \Drupal\Core\Cache\DatabaseBackend.
$this->connection->addSavepoint();
try {
$this->connection->query($stmt, [], $options);
$stmt->execute(NULL, $options);
$this->connection->releaseSavepoint();
}
catch (\Exception $e) {

View File

@ -6,6 +6,7 @@ use Drupal\Core\Database\Connection;
use Drupal\Core\Database\Database;
use Drupal\Core\Database\DatabaseExceptionWrapper;
use Drupal\Core\Database\Query\Condition;
use Drupal\Core\Database\StatementWrapper;
/**
* Tests of the core database system.
@ -134,6 +135,36 @@ class ConnectionTest extends DatabaseTestBase {
$this->assertTrue($foo_connection->supportsTransactions());
}
/**
* Tests the deprecation of passing a statement object to ::query.
*
* @group legacy
*/
public function testStatementQueryDeprecation(): void {
$this->expectDeprecation('Passing a StatementInterface object as a $query argument to Drupal\Core\Database\Connection::query is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Call the execute method from the StatementInterface object directly instead. See https://www.drupal.org/node/3154439');
$db = Database::getConnection();
$stmt = $db->prepareStatement('SELECT * FROM {test}', []);
$this->assertNotNull($db->query($stmt));
}
/**
* Tests the deprecation of passing a PDOStatement object to ::query.
*
* @group legacy
*/
public function testPDOStatementQueryDeprecation(): void {
$db = Database::getConnection();
$stmt = $db->prepareStatement('SELECT * FROM {test}', []);
if (!$stmt instanceof StatementWrapper) {
$this->markTestSkipped("This test only runs for db drivers using StatementWrapper.");
}
if (!$stmt->getClientStatement() instanceof \PDOStatement) {
$this->markTestSkipped("This test only runs for PDO-based db drivers.");
}
$this->expectDeprecation('Passing a \\PDOStatement object as a $query argument to Drupal\Core\Database\Connection::query is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Call the execute method from the StatementInterface object directly instead. See https://www.drupal.org/node/3154439');
$this->assertNotNull($db->query($stmt->getClientStatement()));
}
/**
* Ensure that you cannot execute multiple statements on MySQL.
*/