Unverified Commit 6aab2237 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3137883 by mondrake, Pooja Ganjage, andypost, paulocs, anmolgoyal74,...

Issue #3137883 by mondrake, Pooja Ganjage, andypost, paulocs, anmolgoyal74, daffie, alexpott, catch: Deprecate passing a StatementInterface object to Connection::query
parent b3d61bd3
Loading
Loading
Loading
Loading
+13 −8
Original line number Diff line number Diff line
@@ -749,11 +749,12 @@ protected function filterComment($comment = '') {
   * 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 @@ public function query($query, array $args = [], $options = []) {
    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 @@ protected function handleQueryException(\PDOException $e, $query, array $args =
      // 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();
      }
+27 −16
Original line number Diff line number Diff line
@@ -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 @@ public function execute() {
      }
    }

    // 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 @@ public function execute() {
    // 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);
      // 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 = $this->connection->query($stmt, [], $options);
          $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;
+3 −2
Original line number Diff line number Diff line
@@ -75,9 +75,10 @@ public function execute() {

    $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();
+1 −1
Original line number Diff line number Diff line
@@ -80,7 +80,7 @@ public function execute() {
    // example, \Drupal\Core\Cache\DatabaseBackend.
    $this->connection->addSavepoint();
    try {
      $this->connection->query($stmt, [], $options);
      $stmt->execute(NULL, $options);
      $this->connection->releaseSavepoint();
    }
    catch (\Exception $e) {
+31 −0
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@
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 @@ public function testTransactionsOptionDeprecation() {
    $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.
   */