#890994 by Berdir: Entity query tests fail on PostgreSQL.

5 files changed, 105 insertions, 10 deletions

diff --git a/includes/database/database.inc b/includes/database/database.inc
index 31a4fa645..3e24225da 100644
--- a/includes/database/database.inc
+++ b/includes/database/database.inc
@@ -815,6 +815,7 @@ abstract class DatabaseConnection extends PDO {
    * Force all alias names to be strictly alphanumeric-plus-underscore. In
    * contrast to DatabaseConnection::escapeField() /
    * DatabaseConnection::escapeTable(), this doesn't allow the period (".")
+   * because that is not allowed in aliases.
    *
    * @return
    *   The sanitized field name string.
diff --git a/includes/database/pgsql/query.inc b/includes/database/pgsql/query.inc
index 61c821801..2214d668a 100644
--- a/includes/database/pgsql/query.inc
+++ b/includes/database/pgsql/query.inc
@@ -217,4 +217,86 @@ class SelectQuery_pgsql extends SelectQuery {
     return $this;
   }
 
+  /**
+   * Overrides SelectQuery::orderBy().
+   *
+   * PostgreSQL adheres strictly to the SQL-92 standard and requires that when
+   * using DISTINCT or GROUP BY conditions, fields and expressions that are
+   * ordered on also need to be selected. This is a best effort implementation
+   * to handle the cases that can be automated by adding the field if it is not
+   * yet selected.
+   *
+   * @code
+   *   $query = db_select('node', 'n');
+   *   $query->join('node_revision', 'nr', 'n.vid = nr.vid');
+   *   $query
+   *     ->distinct()
+   *     ->fields('n')
+   *     ->orderBy('timestamp');
+   * @endcode
+   *
+   * In this query, it is not possible (without relying on the schema) to know
+   * whether timestamp belongs to node_revisions and needs to be added or
+   * belongs to node and is already selected. Queries like this will need to be
+   * corrected in the original query by adding an explicit call to
+   * SelectQuery::addField() or SelectQuery::fields().
+   *
+   * Since this has a small performance impact, both by the additional
+   * processing in this function and in the database that needs to return the
+   * additional fields, this is done as an override instead of implementing it
+   * directly in SelectQuery::orderBy().
+   */
+  public function orderBy($field, $direction = 'ASC') {
+    // Call parent function to order on this.
+    $return = parent::orderBy($field, $direction);
+
+    // If there is a table alias specified, split it up.
+    if (strpos($field, '.') !== FALSE) {
+      list($table, $table_field) = explode('.', $field);
+    }
+    // Figure out if the field has already been added.
+    foreach ($this->fields as $existing_field) {
+      if (!empty($table)) {
+        // If table alias is given, check if field and table exists.
+        if ($existing_field['table'] == $table && $existing_field['field'] == $table_field) {
+          return $return;
+        }
+      }
+      else {
+        // If there is no table, simply check if the field exists as a field or
+        // an aliased field.
+        if ($existing_field['alias'] == $field) {
+          return $return;
+        }
+      }
+    }
+
+    // Also check expression aliases.
+    foreach ($this->expressions as $expression) {
+      if ($expression['alias'] == $field) {
+        return $return;
+      }
+    }
+
+    // If a table loads all fields, it can not be added again. It would
+    // result in an ambigious alias error because that field would be loaded
+    // twice: Once through table_alias.* and once directly. If the field
+    // actually belongs to a different table, it must be added manually.
+    foreach ($this->tables as $table) {
+      if (!empty($table['all_fields'])) {
+        return $return;
+      }
+    }
+
+    // If $field contains an characters which are not allowed in a field name
+    // it is considered an expression, these can't be handeld automatically
+    // either.
+    if ($this->connection->escapeField($field) != $field) {
+      return $return;
+    }
+
+    // This is a case that can be handled automatically, add the field.
+    $this->addField(NULL, $field);
+    return $return;
+  }
 }
diff --git a/includes/database/select.inc b/includes/database/select.inc
index 3c0352dd4..72d3454b6 100644
--- a/includes/database/select.inc
+++ b/includes/database/select.inc
@@ -366,6 +366,13 @@ interface SelectQueryInterface extends QueryConditionInterface, QueryAlterableIn
    * If called multiple times, the query will order by each specified field in the
    * order this method is called.
    *
+   * If the query uses DISTINCT or GROUP BY conditions, fields or expressions
+   * that are used for the order must be selected to be compatible with some
+   * databases like PostgreSQL. The PostgreSQL driver can handle simple cases
+   * automatically but it is suggested to explicitly specify them. Additionally,
+   * when ordering on an alias, the alias must be added before orderBy() is
+   * called.
+   *
    * @param $field
    *   The field on which to order.
    * @param $direction
@@ -1419,10 +1426,10 @@ class SelectQuery extends Query implements SelectQueryInterface {
     foreach ($this->fields as $alias => $field) {
       // Always use the AS keyword for field aliases, as some
       // databases require it (e.g., PostgreSQL).
-      $fields[] = (isset($field['table']) ? $this->connection->escapeTable($field['table']) . '.' : '') . $this->connection->escapeField($field['field']) . ' AS ' . $this->connection->escapeField($field['alias']);
+      $fields[] = (isset($field['table']) ? $this->connection->escapeTable($field['table']) . '.' : '') . $this->connection->escapeField($field['field']) . ' AS ' . $this->connection->escapeAlias($field['alias']);
     }
     foreach ($this->expressions as $alias => $expression) {
-      $fields[] = $expression['expression'] . ' AS ' . $expression['alias'];
+      $fields[] = $expression['expression'] . ' AS ' . $this->connection->escapeAlias($expression['alias']);
     }
     $query .= implode(', ', $fields);
 
diff --git a/modules/search/search.extender.inc b/modules/search/search.extender.inc
index 1eae2caa6..6c0c7600b 100644
--- a/modules/search/search.extender.inc
+++ b/modules/search/search.extender.inc
@@ -415,10 +415,6 @@ class SearchQuery extends SelectQueryExtender {
       // Add default score.
       $this->addScore('i.relevance');
     }
-    if (count($this->getOrderBy()) == 0) {
-      // Add default order.
-      $this->orderBy('calculated_score', 'DESC');
-    }
 
     if (count($this->multiply)) {
       // Add the total multiplicator as many times as requested to maintain
@@ -436,6 +432,11 @@ class SearchQuery extends SelectQueryExtender {
     // Convert scores to an expression.
     $this->addExpression('SUM(' . implode(' + ', $this->scores) . ')', 'calculated_score', $this->scoresArguments);
 
+    if (count($this->getOrderBy()) == 0) {
+      // Add default order after adding the expression.
+      $this->orderBy('calculated_score', 'DESC');
+    }
+
     // Add tag and useful metadata.
     $this
       ->addTag('search_' . $this->type)
diff --git a/modules/simpletest/tests/entity_query.test b/modules/simpletest/tests/entity_query.test
index 3660082ed..e569cba1a 100644
--- a/modules/simpletest/tests/entity_query.test
+++ b/modules/simpletest/tests/entity_query.test
@@ -652,16 +652,20 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
     $query = new EntityFieldQuery();
     $query
       ->entityCondition('entity_type', 'test_entity_bundle_key')
-      ->propertyCondition('ftid', 1, 'CONTAINS');
+      ->propertyCondition('fttype', 'und', 'CONTAINS');
     $this->assertEntityFieldQuery($query, array(
       array('test_entity_bundle_key', 1),
+      array('test_entity_bundle_key', 2),
+      array('test_entity_bundle_key', 3),
+      array('test_entity_bundle_key', 4),
+      array('test_entity_bundle_key', 5),
+      array('test_entity_bundle_key', 6),
     ), t('Test the "contains" operation on a property.'));
 
     $query = new EntityFieldQuery();
-    $query->fieldCondition($this->fields[0], 'value', 3, 'CONTAINS');
+    $query->fieldCondition($this->fields[1], 'shape', 'uar', 'CONTAINS');
     $this->assertEntityFieldQuery($query, array(
-      array('test_entity_bundle_key', 3),
-      array('test_entity', 3),
+      array('test_entity_bundle', 5),
     ), t('Test the "contains" operation on a field.'));
 
     $query = new EntityFieldQuery();