Issue #2477853 by ingaro, daffie, bzrudi71, mradcliffe, alexpott: PostgreSQL:...

Issue #2477853 by ingaro, daffie, bzrudi71, mradcliffe, alexpott: PostgreSQL: Add support for reserved field/column names

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

@@ -31,6 +31,27 @@ class Connection extends DatabaseConnection {
    */
   const DATABASE_NOT_FOUND = 7;
 
+  /**
+   * The list of PostgreSQL reserved key words.
+   *
+   * @see http://www.postgresql.org/docs/9.4/static/sql-keywords-appendix.html
+   */
+  protected $postgresqlReservedKeyWords = ['all', 'analyse', 'analyze', 'and',
+  'any', 'array', 'as', 'asc', 'asymmetric', 'authorization', 'binary', 'both',
+  'case', 'cast', 'check', 'collate', 'collation', 'column', 'concurrently',
+  'constraint', 'create', 'cross', 'current_catalog', 'current_date',
+  'current_role', 'current_schema', 'current_time', 'current_timestamp',
+  'current_user', 'default', 'deferrable', 'desc', 'distinct', 'do', 'else',
+  'end', 'except', 'false', 'fetch', 'for', 'foreign', 'freeze', 'from', 'full',
+  'grant', 'group', 'having', 'ilike', 'in', 'initially', 'inner', 'intersect',
+  'into', 'is', 'isnull', 'join', 'lateral', 'leading', 'left', 'like', 'limit',
+  'localtime', 'localtimestamp', 'natural', 'not', 'notnull', 'null', 'offset',
+  'on', 'only', 'or', 'order', 'outer', 'over', 'overlaps', 'placing',
+  'primary', 'references', 'returning', 'right', 'select', 'session_user',
+  'similar', 'some', 'symmetric', 'table', 'then', 'to', 'trailing', 'true',
+  'union', 'unique', 'user', 'using', 'variadic', 'verbose', 'when', 'where',
+  'window', 'with'];
+
   /**
    * Constructs a connection object.
    */
@@ -167,6 +188,10 @@ public function escapeField($field) {
       // Quote the field name for case-sensitivity.
       $escaped = '"' . $escaped . '"';
     }
+    elseif (in_array(strtolower($escaped), $this->postgresqlReservedKeyWords)) {
+      // Quote the field name for PostgreSQL reserved key words.
+      $escaped = '"' . $escaped . '"';
+    }
 
     return $escaped;
   }
@@ -181,6 +206,10 @@ public function escapeAlias($field) {
     if (preg_match('/[A-Z]/', $escaped)) {
       $escaped = '"' . $escaped . '"';
     }
+    elseif (in_array(strtolower($escaped), $this->postgresqlReservedKeyWords)) {
+      // Quote the alias name for PostgreSQL reserved key words.
+      $escaped = '"' . $escaped . '"';
+    }
 
     return $escaped;
   }
@@ -195,6 +224,10 @@ public function escapeTable($table) {
     if (preg_match('/[A-Z]/', $escaped)) {
       $escaped = '"' . $escaped . '"';
     }
+    elseif (in_array(strtolower($escaped), $this->postgresqlReservedKeyWords)) {
+      // Quote the table name for PostgreSQL reserved key words.
+      $escaped = '"' . $escaped . '"';
+    }
 
     return $escaped;
   }

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

@@ -117,6 +117,8 @@ public function __toString() {
     // Default fields are always placed first for consistency.
     $insert_fields = array_merge($this->defaultFields, $this->insertFields);
 
+    $insert_fields = array_map(function($f) { return $this->connection->escapeField($f); }, $insert_fields);
+
     // If we're selecting from a SelectQuery, finish building the query and
     // pass it back, as any remaining options are irrelevant.
     if (!empty($this->fromQuery)) {
@@ -154,4 +156,5 @@ public function __toString() {
 
     return $query;
   }
+
 }

core/lib/Drupal/Core/Database/Query/Update.php

@@ -256,13 +256,13 @@ public function __toString() {
         $data['expression']->compile($this->connection, $this);
         $data['expression'] = ' (' . $data['expression'] . ')';
       }
-      $update_fields[] = $field . '=' . $data['expression'];
+      $update_fields[] = $this->connection->escapeField($field) . '=' . $data['expression'];
       unset($fields[$field]);
     }
 
     $max_placeholder = 0;
     foreach ($fields as $field => $value) {
-      $update_fields[] = $field . '=:db_update_placeholder_' . ($max_placeholder++);
+      $update_fields[] = $this->connection->escapeField($field) . '=:db_update_placeholder_' . ($max_placeholder++);
     }
 
     $query = $comments . 'UPDATE {' . $this->connection->escapeTable($this->table) . '} SET ' . implode(', ', $update_fields);

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

@@ -138,4 +138,23 @@ public function testMultipleStatementsForNewPhp() {
     }
   }
 
+  /**
+   * Test the escapeTable(), escapeField() and escapeAlias() methods with all possible reserved words in PostgreSQL.
+   */
+  public function testPostgresqlReservedWords() {
+    if (Database::getConnection()->databaseType() !== 'pgsql') {
+      return;
+    }
+
+    $db = Database::getConnection('default', 'default');
+    $stmt = $db->query("SELECT word FROM pg_get_keywords() WHERE catcode IN ('R', 'T')");
+    $stmt->execute();
+    foreach ($stmt->fetchAllAssoc('word') as $word => $row) {
+      $expected = '"' . $word . '"';
+      $this->assertIdentical($db->escapeTable($word), $expected, format_string('The reserved word %word was correctly escaped when used as a table name.', array('%word' => $word)));
+      $this->assertIdentical($db->escapeField($word), $expected, format_string('The reserved word %word was correctly escaped when used as a column name.', array('%word' => $word)));
+      $this->assertIdentical($db->escapeAlias($word), $expected, format_string('The reserved word %word was correctly escaped when used as an alias.', array('%word' => $word)));
+    }
+  }
+
 }

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

@@ -30,6 +30,7 @@ protected function setUp() {
       'test_task',
       'test_null',
       'test_serialized',
+      'test_special_columns',
     ));
     self::addSampleData();
   }
@@ -138,5 +139,12 @@ static function addSampleData() {
         'priority' => 3,
       ))
       ->execute();
+
+    db_insert('test_special_columns')
+      ->fields(array(
+        'id' => 1,
+        'offset' => 'Offset value 1',
+      ))
+      ->execute();
   }
 }

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

@@ -70,4 +70,19 @@ function testTruncate() {
     $num_records_after = db_query("SELECT COUNT(*) FROM {test}")->fetchField();
     $this->assertEqual(0, $num_records_after, 'Truncate really deletes everything.');
   }
+
+  /**
+   * Confirms that we can delete a single special column name record successfully.
+   */
+  function testSpecialColumnDelete() {
+    $num_records_before = db_query('SELECT COUNT(*) FROM {test_special_columns}')->fetchField();
+
+    $num_deleted = db_delete('test_special_columns')
+      ->condition('id', 1)
+      ->execute();
+    $this->assertIdentical($num_deleted, 1, 'Deleted 1 special column record.');
+
+    $num_records_after = db_query('SELECT COUNT(*) FROM {test_special_columns}')->fetchField();
+    $this->assertEqual($num_records_before, $num_records_after + $num_deleted, 'Deletion adds up.');
+  }
 }

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

@@ -182,4 +182,17 @@ function testInsertSelectAll() {
     $this->assertIdentical($saved_age, '30', 'Can retrieve after inserting.');
   }
 
+  /**
+   * Tests that we can INSERT INTO a special named column.
+   */
+  function testSpecialColumnInsert() {
+    $id = db_insert('test_special_columns')
+      ->fields(array(
+        'id' => 2,
+        'offset' => 'Offset value 2',
+      ))
+      ->execute();
+    $saved_value = db_query('SELECT "offset" FROM {test_special_columns} WHERE id = :id', array(':id' => 2))->fetchField();
+    $this->assertIdentical($saved_value, 'Offset value 2', 'Can retrieve special column name value after inserting.');
+  }
 }

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

@@ -145,4 +145,18 @@ function testPrimaryKeyUpdate() {
     $saved_name= db_query('SELECT name FROM {test} WHERE id = :id', array(':id' => 42))->fetchField();
     $this->assertIdentical($saved_name, 'John', 'Updated primary key successfully.');
   }
+
+  /**
+   * Confirm that we can update values in a column with special name.
+   */
+  function testSpecialColumnUpdate() {
+    $num_updated = db_update('test_special_columns')
+      ->fields(array('offset' => 'New offset value'))
+      ->condition('id', 1)
+      ->execute();
+    $this->assertIdentical($num_updated, 1, 'Updated 1 special column record.');
+
+    $saved_value = db_query('SELECT "offset" FROM {test_special_columns} WHERE id = :id', array(':id' => 1))->fetchField();
+    $this->assertIdentical($saved_value, 'New offset value', 'Updated special column name value successfully.');
+  }
 }

core/modules/system/tests/modules/database_test/database_test.install

@@ -274,5 +274,21 @@ function database_test_schema() {
     'primary key' => array('name', 'age'),
   );
 
+  $schema['test_special_columns'] = array(
+    'description' => 'A simple test table with special column names.',
+    'fields' => array(
+      'id' => array(
+        'description' => 'Simple unique ID.',
+        'type' => 'int',
+        'not null' => TRUE,
+      ),
+      'offset' => array(
+        'description' => 'A column with preserved name.',
+        'type' => 'text',
+      ),
+    ),
+    'primary key' => array('id'),
+  );
+
   return $schema;
 }