Verified Commit f0a52234 authored by Dave Long's avatar Dave Long
Browse files

refactor: #3615415 Remove using preg_replace() on every query for PostgreSQL

By: daffie
By: smustgrave
By: mradcliffe
parent 0d32f86c
Loading
Loading
Loading
Loading
Loading
+36 −1
Changes for core/lib/Drupal/Core/Database/Connection.php: 36 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -1265,12 +1265,47 @@ abstract public function createDatabase($database);
   *   The condition operator, such as "IN", "BETWEEN", etc. Case-sensitive.
   *
   * @return array|null
   *   The extra handling directives for the specified operator, or NULL.
   *   The extra handling directives for the specified operator, or NULL. The
   *   directives are an array with any of the following keys:
   *   - operator: The replacement operator.
   *   - field_suffix: SQL to append to the field, such as a type cast.
   *   - prefix: SQL to add before the value.
   *   - postfix: SQL to add after the value.
   *   - delimiter: The delimiter to implode multiple values with.
   *   - use_value: Whether to compile the value part. Defaults to TRUE.
   *
   * @see \Drupal\Core\Database\Query\Condition::compile()
   */
  abstract public function mapConditionOperator($operator);

  /**
   * Gets the field suffix for the specified condition operator.
   *
   * Only needed when a condition is built as an SQL snippet, for example
   * with where(). Conditions added with condition() get the suffix added
   * when the condition is compiled.
   *
   * For example, on PostgreSQL the LIKE operator needs a '::text' type-cast
   * on the field:
   * @code
   * $suffix = $connection->getConditionFieldSuffix('LIKE');
   * $query->where("[title]$suffix LIKE :pattern", [
   *   ':pattern' => '%foo%',
   * ]);
   * @endcode
   *
   * @param string $operator
   *   The condition operator, such as "LIKE", "REGEXP", etc. Case-sensitive.
   *
   * @return string
   *   The SQL the database driver needs appended to the field for this
   *   operator, for example a type-cast, or an empty string.
   */
  public function getConditionFieldSuffix(string $operator): string {
    $mapping = $this->mapConditionOperator($operator);
    return $mapping['field_suffix'] ?? '';
  }

  /**
   * Quotes a string for use in a query.
   *
+6 −1
Changes for core/lib/Drupal/Core/Database/Query/Condition.php: 6 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -272,6 +272,7 @@ public function compile(Connection $connection, PlaceholderInterface $queryPlace
        }
        // Add defaults.
        $operator += [
          'field_suffix' => '',
          'prefix' => '',
          'postfix' => '',
          'delimiter' => '',
@@ -316,7 +317,11 @@ public function compile(Connection $connection, PlaceholderInterface $queryPlace
        }

        // Concatenate the left hand part, operator and right hand part.
        $condition_fragments[] = trim(implode(' ', [$field_fragment, $operator_fragment, $value_fragment]));
        $condition_fragments[] = trim(implode(' ', [
          $field_fragment . $operator['field_suffix'],
          $operator_fragment,
          $value_fragment,
        ]));
      }

      // Concatenate all conditions using the conjunction and brackets around
+31 −12
Changes for core/modules/pgsql/src/Driver/Database/pgsql/Connection.php: 31 added lines, 12 removed lines.
Original line number Diff line number Diff line
@@ -13,7 +13,7 @@
use Drupal\Core\Database\Transaction\TransactionManagerInterface;
use Pdo\Pgsql;

// cSpell:ignore ilike nextval
// cSpell:ignore ilike nextval uncast

/**
 * @addtogroup database
@@ -52,16 +52,25 @@ class Connection extends DatabaseConnection implements SupportsTemporaryTablesIn
   * A map of condition operators to PostgreSQL operators.
   *
   * In PostgreSQL, 'LIKE' is case-sensitive. ILIKE should be used for
   * case-insensitive statements.
   * case-insensitive statements. PostgreSQL doesn't automatically cast
   * fields to the right type for these operators, so the fields are cast
   * to text.
   *
   * @var string[][]
   */
  protected static $postgresqlConditionOperatorMap = [
    'LIKE' => ['operator' => 'ILIKE'],
    'LIKE BINARY' => ['operator' => 'LIKE'],
    'NOT LIKE' => ['operator' => 'NOT ILIKE'],
    'REGEXP' => ['operator' => '~*'],
    'NOT REGEXP' => ['operator' => '!~*'],
    'LIKE' => ['operator' => 'ILIKE', 'field_suffix' => '::text'],
    'LIKE BINARY' => ['operator' => 'LIKE', 'field_suffix' => '::text'],
    'NOT LIKE' => ['operator' => 'NOT ILIKE', 'field_suffix' => '::text'],
    'REGEXP' => ['operator' => '~*', 'field_suffix' => '::text'],
    'NOT REGEXP' => ['operator' => '!~*', 'field_suffix' => '::text'],
    // Identity entries, so that conditions built with PostgreSQL operators
    // directly also get the field cast. For example, Views' StringFilter
    // maps 'LIKE' to 'ILIKE' itself before adding the condition.
    'ILIKE' => ['operator' => 'ILIKE', 'field_suffix' => '::text'],
    'NOT ILIKE' => ['operator' => 'NOT ILIKE', 'field_suffix' => '::text'],
    '~*' => ['operator' => '~*', 'field_suffix' => '::text'],
    '!~*' => ['operator' => '!~*', 'field_suffix' => '::text'],
  ];

  /**
@@ -253,11 +262,21 @@ public function query($query, array $args = [], $options = []) {
   * {@inheritdoc}
   */
  public function prepareStatement(string $query, array $options, bool $allow_row_count = FALSE): StatementInterface {
    // mapConditionOperator converts some operations (LIKE, REGEXP, etc.) to
    // PostgreSQL equivalents (ILIKE, ~*, etc.). However PostgreSQL doesn't
    // automatically cast the fields to the right type for these operators,
    // so we need to alter the query and add the type-cast.
    $query = preg_replace('/ ([^ ]+) +(I*LIKE|NOT +I*LIKE|~\*|!~\*) /i', ' ${1}::text ${2} ', $query);
    // PostgreSQL doesn't automatically cast fields to the right type for the
    // (I)LIKE and regex operators. Query builder queries get a '::text'
    // type-cast when their conditions are compiled (see
    // static::$postgresqlConditionOperatorMap), but queries passed as raw SQL
    // strings may still contain uncast fields, so add the type-cast here. The
    // guard skips the rewrite for the many queries that use neither operator.
    // The optional '::text' in the pattern makes the rewrite idempotent, so
    // fields that are already cast are not cast again.
    if (stripos($query, 'LIKE') !== FALSE || str_contains($query, '~*')) {
      $rewritten = preg_replace('/ ([^ ]+?)(?:::text)? +(I*LIKE|NOT +I*LIKE|~\*|!~\*) /i', ' ${1}::text ${2} ', $query);
      if ($rewritten !== $query) {
        @trigger_error('Relying on the PostgreSQL database driver to add a ::text type-cast to fields used with LIKE and regular expression operators in SQL passed as a string is deprecated in drupal:11.5.0 and is removed from drupal:13.0.0. Add the ::text cast to the field explicitly or use the query builder. See https://www.drupal.org/node/3615418', E_USER_DEPRECATED);
        $query = $rewritten;
      }
    }
    return parent::prepareStatement($query, $options, $allow_row_count);
  }

+1 −1
Changes for core/modules/pgsql/src/Driver/Database/pgsql/Schema.php: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -167,7 +167,7 @@ public function queryTableInformation($table) {
AND NOT pg_attribute.attisdropped
AND pg_attribute.attrelid = :key::regclass
AND (format_type(pg_attribute.atttypid, pg_attribute.atttypmod) = 'bytea'
OR pg_get_expr(pg_attrdef.adbin, pg_attribute.attrelid) LIKE 'nextval%')
OR pg_get_expr(pg_attrdef.adbin, pg_attribute.attrelid)::text LIKE 'nextval%')
EOD;
        $result = $this->connection->query($sql, [
          ':key' => $quoted_key,
+90 −0
Changes for core/modules/pgsql/tests/src/Kernel/pgsql/ConditionCastTest.php: 90 added lines, 0 removed lines.
Original line number Diff line number Diff line
<?php

declare(strict_types=1);

namespace Drupal\Tests\pgsql\Kernel\pgsql;

use Drupal\KernelTests\Core\Database\DriverSpecificDatabaseTestBase;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
use PHPUnit\Framework\Attributes\RunTestsInSeparateProcesses;

// cSpell:ignore ilike

/**
 * Tests casting of non-text fields for LIKE and regex operators.
 */
#[Group('Database')]
#[RunTestsInSeparateProcesses]
class ConditionCastTest extends DriverSpecificDatabaseTestBase {

  /**
   * Tests condition operators on an integer column.
   *
   * PostgreSQL has no LIKE or regex operators for integers, so these queries
   * only work when the field is cast to text. For query builder conditions
   * the cast is added when the condition is compiled.
   *
   * @see \Drupal\pgsql\Driver\Database\pgsql\Connection::$postgresqlConditionOperatorMap
   */
  public function testConditionOperatorCastOnIntegerField(): void {
    // The ages in the sample data are 25, 27, 28 and 26.
    $cases = [
      ['LIKE', '2%', 4],
      ['LIKE', '%5', 1],
      ['NOT LIKE', '%5', 3],
      ['LIKE BINARY', '%7', 1],
      ['ILIKE', '%6', 1],
      ['NOT ILIKE', '%6', 3],
      ['REGEXP', '^2[57]$', 2],
      ['NOT REGEXP', '^2[57]$', 2],
      ['~*', '8$', 1],
      ['!~*', '8$', 3],
    ];
    foreach ($cases as [$operator, $value, $expected_count]) {
      $query = $this->connection->select('test');
      $query->addField('test', 'name');
      $query->condition('age', $value, $operator);
      $this->assertStringContainsString('::text', (string) $query, "The compiled condition for operator '$operator' contains a type-cast.");
      $this->assertCount($expected_count, $query->execute()->fetchAll(), "Operator '$operator' with value '$value' returns $expected_count rows.");
    }
  }

  /**
   * Tests LIKE and regex operators in raw SQL on an integer column.
   *
   * SQL passed as a string does not go through condition compilation, so
   * prepareStatement() adds the '::text' cast to these queries. Relying on
   * this rewrite is deprecated.
   *
   * @see \Drupal\pgsql\Driver\Database\pgsql\Connection::prepareStatement()
   */
  #[IgnoreDeprecations]
  public function testRawSqlOperatorCastOnIntegerField(): void {
    $this->expectUserDeprecationMessage('Relying on the PostgreSQL database driver to add a ::text type-cast to fields used with LIKE and regular expression operators in SQL passed as a string is deprecated in drupal:11.5.0 and is removed from drupal:13.0.0. Add the ::text cast to the field explicitly or use the query builder. See https://www.drupal.org/node/3615418');

    $statement = $this->connection->prepareStatement('SELECT COUNT(*) FROM {test} WHERE [age] LIKE :pattern', []);
    $this->assertStringContainsString('::text', $statement->getQueryString());

    $count = $this->connection->query('SELECT COUNT(*) FROM {test} WHERE [age] LIKE :pattern', [':pattern' => '2%'])->fetchField();
    $this->assertSame(4, (int) $count);

    $count = $this->connection->query('SELECT COUNT(*) FROM {test} WHERE [age] ~* :pattern', [':pattern' => '8$'])->fetchField();
    $this->assertSame(1, (int) $count);

    // A raw condition snippet inside a query builder query.
    $query = $this->connection->select('test');
    $query->addField('test', 'name');
    $query->where('[age] LIKE :pattern', [':pattern' => '%6']);
    $this->assertCount(1, $query->execute()->fetchAll());
  }

  /**
   * Tests that raw SQL with an explicit cast does not trigger a deprecation.
   */
  public function testRawSqlWithExplicitCast(): void {
    $count = $this->connection->query('SELECT COUNT(*) FROM {test} WHERE [age]::text LIKE :pattern', [':pattern' => '2%'])->fetchField();
    $this->assertSame(4, (int) $count);
  }

}
Loading