Issue #1600670 by mradcliffe, bendiy, bzrudi71, andypost, daffie, stefan.r,...

Issue #1600670 by mradcliffe, bendiy, bzrudi71, andypost, daffie, stefan.r, devpreview: Cannot query Postgres database that has column names with capital letters

core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php

@@ -186,6 +186,64 @@ public function queryTemporary($query, array $args = array(), array $options = a
     return $tablename;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function escapeField($field) {
+    $escaped = parent::escapeField($field);
+
+    // Remove any invalid start character.
+    $escaped = preg_replace('/^[^A-Za-z0-9_]/', '', $escaped);
+
+    // The pgsql database driver does not support field names that contain
+    // periods (supported by PostgreSQL server) because this method may be
+    // called by a field with a table alias as part of SQL conditions or
+    // order by statements. This will consider a period as a table alias
+    // identifier, and split the string at the first period.
+    if (preg_match('/^([A-Za-z0-9_]+)"?[.]"?([A-Za-z0-9_.]+)/', $escaped, $parts)) {
+      $table = $parts[1];
+      $column = $parts[2];
+
+      // Use escape alias because escapeField may contain multiple periods that
+      // need to be escaped.
+      $escaped = $this->escapeTable($table) . '.' . $this->escapeAlias($column);
+    }
+    elseif (preg_match('/[A-Z]/', $escaped)) {
+      // Quote the field name for case-sensitivity.
+      $escaped = '"' . $escaped . '"';
+    }
+
+    return $escaped;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function escapeAlias($field) {
+    $escaped = preg_replace('/[^A-Za-z0-9_]+/', '', $field);
+
+    // Escape the alias in quotes for case-sensitivity.
+    if (preg_match('/[A-Z]/', $escaped)) {
+      $escaped = '"' . $escaped . '"';
+    }
+
+    return $escaped;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function escapeTable($table) {
+    $escaped = parent::escapeTable($table);
+
+    // Quote identifier to make it case-sensitive.
+    if (preg_match('/[A-Z]/', $escaped)) {
+      $escaped = '"' . $escaped . '"';
+    }
+
+    return $escaped;
+  }
+
   public function driver() {
     return 'pgsql';
   }

core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php

@@ -262,7 +262,8 @@ protected function createTableSql($name, $table) {
    *    The field specification, as per the schema data structure format.
    */
   protected function createFieldSql($name, $spec) {
-    $sql = $name . ' ' . $spec['pgsql_type'];
+    // The PostgreSQL server converts names into lowercase, unless quoted.
+    $sql = '"' . $name . '" ' . $spec['pgsql_type'];
 
     if (isset($spec['type']) && $spec['type'] == 'serial') {
       unset($spec['not null']);

core/lib/Drupal/Core/Database/Driver/pgsql/Select.php

@@ -82,7 +82,7 @@ public function orderBy($field, $direction = 'ASC') {
 
     // Also check expression aliases.
     foreach ($this->expressions as $expression) {
-      if ($expression['alias'] == $field) {
+      if ($expression['alias'] == $this->connection->escapeAlias($field)) {
         return $return;
       }
     }
@@ -109,6 +109,31 @@ public function orderBy($field, $direction = 'ASC') {
     return $return;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function addExpression($expression, $alias = NULL, $arguments = array()) {
+    if (empty($alias)) {
+      $alias = 'expression';
+    }
+
+    // This implements counting in the same manner as the parent method.
+    $alias_candidate = $alias;
+    $count = 2;
+    while (!empty($this->expressions[$alias_candidate])) {
+      $alias_candidate = $alias . '_' . $count++;
+    }
+    $alias = $alias_candidate;
+
+    $this->expressions[$alias] = array(
+      'expression' => $expression,
+      'alias' => $this->connection->escapeAlias($alias_candidate),
+      'arguments' => $arguments,
+    );
+
+    return $alias;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/system/src/Tests/Database/SelectTest.php

@@ -39,10 +39,10 @@ function testSimpleComment() {
     $records = $result->fetchAll();
 
     $query = (string) $query;
-    $expected = "/* Testing query comments */ SELECT test.name AS name, test.age AS age\nFROM \n{test} test";
+    $expected = "/* Testing query comments */";
 
     $this->assertEqual(count($records), 4, 'Returned the correct number of rows.');
-    $this->assertEqual($query, $expected, 'The flattened query contains the comment string.');
+    $this->assertNotIdentical(FALSE, strpos($query, $expected), 'The flattened query contains the comment string.');
   }
 
   /**
@@ -57,10 +57,10 @@ function testVulnerableComment() {
     $records = $result->fetchAll();
 
     $query = (string) $query;
-    $expected = "/* Testing query comments SELECT nid FROM {node}; -- */ SELECT test.name AS name, test.age AS age\nFROM \n{test} test";
+    $expected = "/* Testing query comments SELECT nid FROM {node}; -- */";
 
     $this->assertEqual(count($records), 4, 'Returned the correct number of rows.');
-    $this->assertEqual($query, $expected, 'The flattened query contains the sanitised comment string.');
+    $this->assertNotIdentical(FALSE, strpos($query, $expected), 'The flattened query contains the sanitised comment string.');
   }
 
   /**

core/modules/system/src/Tests/Database/UpdateComplexTest.php

@@ -139,15 +139,12 @@ function testSubSelectUpdate() {
     $query = db_update('test')
       ->expression('age', $subselect)
       ->condition('name', 'Ringo');
-    // Save the query for a __toString test later.
-    $string_test = $query;
-    $query->execute();
+    // Save the number of rows that updated for assertion later.
+    $num_updated = $query->execute();
     $after_age = db_query('SELECT age FROM {test} WHERE name = :name', array(':name' => 'Ringo'))->fetchField();
     $expected_age = $select->execute()->fetchField();
     $this->assertEqual($after_age, $expected_age);
-    // Replace whitespace with a single space.
-    $query_string = preg_replace('/\s+/', ' ', $string_test);
-    $this->assertIdentical('UPDATE {test} SET age= (SELECT MAX(priority) + :increment AS max_priority FROM {test_task} t) WHERE (name = :db_condition_placeholder_0)', trim($query_string));
+    $this->assertEqual(1, $num_updated, t('Expected 1 row to be updated in subselect update query.'));
   }
 
 }

core/tests/Drupal/Tests/Core/Database/Driver/pgsql/PostgresqlConnectionTest.php

+<?php
+/**
+ * @file
+ * Contains Drupal\Tests\Core\Database\Driver\pgsql\PostgresqlConnectionTest
+ */
+
+namespace Drupal\Tests\Core\Database\Driver\pgsql;
+
+use Drupal\Core\Database\Driver\pgsql\Connection;
+use Drupal\Tests\Core\Database\Stub\StubPDO;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Database\Driver\pgsql\Connection
+ * @group Database
+ */
+class PostgresqlConnectionTest extends UnitTestCase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+    $this->mock_pdo = $this->getMock('Drupal\Tests\Core\Database\Stub\StubPDO');
+  }
+
+  /**
+   * Data provider for testEscapeTable.
+   *
+   * @return []
+   *   An indexed array of where each value is an array of arguments to pass to
+   *   testEscapeField. The first value is the expected value, and the second
+   *   value is the value to test.
+   */
+  public function providerEscapeTables() {
+    return array(
+      array('nocase', 'nocase'),
+      array('"camelCase"', 'camelCase'),
+      array('"camelCase"', '"camelCase"'),
+      array('"camelCase"', 'camel/Case'),
+    );
+  }
+
+  /**
+   * Data provider for testEscapeAlias.
+   *
+   * @return []
+   *   Array of arrays with the following elements:
+   *   - Expected escaped string.
+   *   - String to escape.
+   */
+  public function providerEscapeAlias() {
+    return array(
+      array('nocase', 'nocase'),
+      array('"camelCase"', '"camelCase"'),
+      array('"camelCase"', 'camelCase'),
+      array('"camelCase"', 'camel.Case'),
+    );
+  }
+
+  /**
+   * Data provider for testEscapeField.
+   *
+   * @return []
+   *   Array of arrays with the following elements:
+   *   - Expected escaped string.
+   *   - String to escape.
+   */
+  public function providerEscapeFields() {
+    return array(
+      array('title', 'title'),
+      array('"isDefaultRevision"', 'isDefaultRevision'),
+      array('"isDefaultRevision"', '"isDefaultRevision"'),
+      array('entity_test."isDefaultRevision"', 'entity_test.isDefaultRevision'),
+      array('entity_test."isDefaultRevision"', '"entity_test"."isDefaultRevision"'),
+      array('"entityTest"."isDefaultRevision"', '"entityTest"."isDefaultRevision"'),
+      array('"entityTest"."isDefaultRevision"', 'entityTest.isDefaultRevision'),
+      array('entity_test."isDefaultRevision"', 'entity_test.is.Default.Revision'),
+    );
+  }
+
+  /**
+   * @covers ::escapeTable
+   * @dataProvider providerEscapeTables
+   */
+  public function testEscapeTable($expected, $name) {
+    $pgsql_connection = new Connection($this->mock_pdo, array());
+
+    $this->assertEquals($expected, $pgsql_connection->escapeTable($name));
+  }
+
+  /**
+   * @covers ::escapeAlias
+   * @dataProvider providerEscapeAlias
+   */
+  public function testEscapeAlias($expected, $name) {
+    $pgsql_connection = new Connection($this->mock_pdo, array());
+
+    $this->assertEquals($expected, $pgsql_connection->escapeAlias($name));
+  }
+
+  /**
+   * @covers ::escapeField
+   * @dataProvider providerEscapeFields
+   */
+  public function testEscapeField($expected, $name) {
+    $pgsql_connection = new Connection($this->mock_pdo, array());
+
+    $this->assertEquals($expected, $pgsql_connection->escapeField($name));
+  }
+
+}