Permalink
Browse files

BUG Fix data query not always joining necessary tables

Fixes #2846
  • Loading branch information...
1 parent bbaf233 commit a3c8a594cad94fcb24e60a11cd2cbf9bc9bdfb7a @tractorcow tractorcow committed Apr 8, 2014
Showing with 59 additions and 24 deletions.
  1. +23 −23 model/DataQuery.php
  2. +3 −1 tests/model/DataObjectLazyLoadingTest.php
  3. +23 −0 tests/model/DataQueryTest.php
  4. +10 −0 tests/model/DataQueryTest.yml
View
46 model/DataQuery.php
@@ -149,11 +149,13 @@ public function getFinalisedQuery($queriedColumns = null) {
}
$query = clone $this->query;
+ $ancestorTables = ClassInfo::ancestry($this->dataClass, true);
- // Generate the list of tables to iterate over and the list of columns required by any existing where clauses.
- // This second step is skipped if we're fetching the whole dataobject as any required columns will get selected
- // regardless.
+ // Generate the list of tables to iterate over and the list of columns required
+ // by any existing where clauses. This second step is skipped if we're fetching
+ // the whole dataobject as any required columns will get selected regardless.
if($queriedColumns) {
+ // Specifying certain columns allows joining of child tables
$tableClasses = ClassInfo::dataClassesFor($this->dataClass);
foreach ($query->getWhere() as $where) {
@@ -163,39 +165,36 @@ public function getFinalisedQuery($queriedColumns = null) {
if (!in_array($matches[1], $queriedColumns)) $queriedColumns[] = $matches[1];
}
}
+ } else {
+ $tableClasses = $ancestorTables;
}
- else $tableClasses = ClassInfo::ancestry($this->dataClass, true);
$tableNames = array_keys($tableClasses);
$baseClass = $tableNames[0];
// Iterate over the tables and check what we need to select from them. If any selects are made (or the table is
// required for a select)
foreach($tableClasses as $tableClass) {
- $joinTable = false;
- // If queriedColumns is set, then check if any of the fields are in this table.
+ // Determine explicit columns to select
+ $selectColumns = null;
if ($queriedColumns) {
+ // Restrict queried columns to that on the selected table
$tableFields = DataObject::database_fields($tableClass);
- $selectColumns = array();
- // Look through columns specifically requested in query (or where clause)
- foreach ($queriedColumns as $queriedColumn) {
- if (array_key_exists($queriedColumn, $tableFields)) {
- $selectColumns[] = $queriedColumn;
- }
- }
-
+ $selectColumns = array_intersect($queriedColumns, array_keys($tableFields));
+ }
+
+ // If this is a subclass without any explicitly requested columns, omit this from the query
+ if(!in_array($tableClass, $ancestorTables) && empty($selectColumns)) continue;
+
+ // Select necessary columns (unless an explicitly empty array)
+ if($selectColumns !== array()) {
$this->selectColumnsFromTable($query, $tableClass, $selectColumns);
- if ($selectColumns && $tableClass != $baseClass) {
- $joinTable = true;
- }
- } else {
- $this->selectColumnsFromTable($query, $tableClass);
- if ($tableClass != $baseClass) $joinTable = true;
}
- if ($joinTable) {
- $query->addLeftJoin($tableClass, "\"$tableClass\".\"ID\" = \"$baseClass\".\"ID\"", $tableClass, 10) ;
+ // Join if not the base table
+ if($tableClass !== $baseClass) {
+ $query->addLeftJoin($tableClass, "\"$tableClass\".\"ID\" = \"$baseClass\".\"ID\"", $tableClass, 10);
}
}
@@ -687,7 +686,8 @@ public function selectFromTable($table, $fields) {
/**
* Query the given field column from the database and return as an array.
*
- * @param String $field See {@link expressionForField()}.
+ * @param string $field See {@link expressionForField()}.
+ * @return array List of column values for the specified column
*/
public function column($field = 'ID') {
$fieldExpression = $this->expressionForField($field);
View
4 tests/model/DataObjectLazyLoadingTest.php
@@ -35,6 +35,7 @@ public function testQueriedColumnsID() {
'"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' .
$db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName", "DataObjectTest_Team"."Title" '.
'FROM "DataObjectTest_Team" ' .
+ 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" ' .
'WHERE ("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').'))' .
' ORDER BY "DataObjectTest_Team"."Title" ASC';
$this->assertEquals($expected, $playerList->sql());
@@ -62,7 +63,8 @@ public function testQueriedColumnsFromBaseTable() {
$expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."Created", ' .
'"DataObjectTest_Team"."LastEdited", "DataObjectTest_Team"."Title", "DataObjectTest_Team"."ID", ' .
'CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' .
- $db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName" FROM "DataObjectTest_Team" WHERE ' .
+ $db->prepStringForDB('DataObjectTest_Team').' END AS "RecordClassName" FROM "DataObjectTest_Team" ' .
+ 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' .
'("DataObjectTest_Team"."ClassName" IN ('.$db->prepStringForDB('DataObjectTest_SubTeam').')) ' .
'ORDER BY "DataObjectTest_Team"."Title" ASC';
$this->assertEquals($expected, $playerList->sql());
View
23 tests/model/DataQueryTest.php
@@ -1,11 +1,15 @@
<?php
class DataQueryTest extends SapphireTest {
+
+ protected static $fixture_file = 'DataQueryTest.yml';
protected $extraDataObjects = array(
'DataQueryTest_A',
'DataQueryTest_B',
+ 'DataQueryTest_C',
'DataQueryTest_D',
+ 'DataQueryTest_E',
);
/**
@@ -124,6 +128,12 @@ public function testOrderByMultiple() {
$dq->sql()
);
}
+
+ public function testDefaultSort() {
+ $query = new DataQuery('DataQueryTest_E');
+ $result = $query->column('Title');
+ $this->assertEquals(array('First', 'Second', 'Last'), $result);
+ }
}
@@ -148,6 +158,10 @@ class DataQueryTest_B extends DataQueryTest_A {
}
class DataQueryTest_C extends DataObject implements TestOnly {
+
+ private static $db = array(
+ 'Title' => 'Varchar'
+ );
private static $has_one = array(
'TestA' => 'DataQueryTest_A',
@@ -171,3 +185,12 @@ class DataQueryTest_D extends DataObject implements TestOnly {
'Relation' => 'DataQueryTest_B',
);
}
+
+class DataQueryTest_E extends DataQueryTest_C implements TestOnly {
+
+ private static $db = array(
+ 'SortOrder' => 'Int'
+ );
+
+ private static $default_sort = '"DataQueryTest_E"."SortOrder" ASC';
+}
View
10 tests/model/DataQueryTest.yml
@@ -0,0 +1,10 @@
+DataQueryTest_E:
+ query1:
+ Title: 'Last'
+ SortOrder: 3
+ query2:
+ Title: 'First'
+ SortOrder: 1
+ query3:
+ Title: 'Second'
+ SortOrder: 2

0 comments on commit a3c8a59

Please sign in to comment.