Skip to content

Commit

Permalink
Fix invalid order clauses being left on subqueries
Browse files Browse the repository at this point in the history
When eager loading associations that contain an ORDER, we need to
augment the selected fields to include the fields being ordered by. If
we were to remove the ORDER BY, incorrect association data would be
loaded. If we were to leave the fields alone, we'd have problems with
postgres. Instead, if we find fields in the ORDER BY, that also exist in
the column list, those fields should be preserved in the generated
subquery.

Refs #7669
  • Loading branch information
markstory committed Nov 11, 2015
1 parent 4345fdf commit 993baf3
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 5 deletions.
39 changes: 34 additions & 5 deletions src/ORM/Association/SelectableAssociationTrait.php
Expand Up @@ -145,7 +145,11 @@ public function _addFilteringJoin($query, $key, $subquery)
$aliasedTable = $this->source()->alias();

foreach ($subquery->clause('select') as $aliasedField => $field) {
$filter[] = new IdentifierExpression($field);
if (is_int($aliasedField)) {
$filter[] = new IdentifierExpression($field);
} else {
$filter[$aliasedField] = $field;
}
}
$subquery->select($filter, true);

Expand Down Expand Up @@ -234,15 +238,40 @@ protected function _buildSubquery($query)
$filterQuery->offset(null);
}

$keys = (array)$this->bindingKey();
$fields = $this->_subqueryFields($query);
$filterQuery->select($fields['select'], true)->group($fields['group']);
return $filterQuery;
}

/**
* Calculate the fields that need to participate in a subquery.
*
* Normally this includes the binding key columns. If there is a an ORDER BY,
* those columns are also included as the fields may be calculated or constant values,
* that need to be present to ensure the correct association data is loaded.
*
* @param \Cake\ORM\Query $query The query to get fields from.
* @return array The list of fields for the subquery.
*/
protected function _subqueryFields($query)
{
$keys = (array)$this->bindingKey();
if ($this->type() === $this::MANY_TO_ONE) {
$keys = (array)$this->foreignKey();
}

$fields = $query->aliasFields($keys, $this->source()->alias());
$filterQuery->select($fields, true)->group(array_values($fields));
return $filterQuery;
$group = $fields = array_values($fields);

$order = $query->clause('order');
if ($order) {
$columns = $query->clause('select');
$order->iterateParts(function($direction, $field) use (&$fields, $columns) {
if (isset($columns[$field])) {
$fields[$field] = $columns[$field];
}
});
}
return ['select' => $fields, 'group' => $group];
}

/**
Expand Down
22 changes: 22 additions & 0 deletions tests/TestCase/ORM/QueryRegressionTest.php
Expand Up @@ -1098,4 +1098,26 @@ public function testComplexOrderWithUnion()
$results = $query->toArray();
$this->assertCount(5, $results);
}

/**
* Test that associations that are loaded with subqueries
* do not cause errors when the subquery has a limit & order clause.
*
* @return void
*/
public function testEagerLoadOrderAndSubquery()
{
$table = TableRegistry::get('Articles');
$table->hasMany('Comments', [
'strategy' => 'subquery'
]);
$query = $table->find()
->select(['score' => 100])
->autoFields(true)
->contain(['Comments'])
->limit(5)
->order(['score' => 'desc']);
$result = $query->all();
$this->assertCount(3, $result);
}
}

0 comments on commit 993baf3

Please sign in to comment.