From decd7e5c57a0839cb873c86782a870326ffa175a Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Sat, 21 May 2016 12:56:51 +1200 Subject: [PATCH] BUG Fix getFinalisedQuery not including all queried columns Fixes #1669 --- model/DataQuery.php | 20 +++++++++---- tests/model/DataQueryTest.php | 53 ++++++++++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/model/DataQuery.php b/model/DataQuery.php index e82bcf1ee..fe7cb7d62 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -170,7 +170,9 @@ class DataQuery { * @return SQLQuery The finalised sql query */ public function getFinalisedQuery($queriedColumns = null) { - if(!$queriedColumns) $queriedColumns = $this->queriedColumns; + if(!$queriedColumns) { + $queriedColumns = $this->queriedColumns; + } if($queriedColumns) { $queriedColumns = array_merge($queriedColumns, array('Created', 'LastEdited', 'ClassName')); } @@ -185,11 +187,19 @@ class DataQuery { // Specifying certain columns allows joining of child tables $tableClasses = ClassInfo::dataClassesFor($this->dataClass); + // Ensure that any filtered columns are included in the selected columns foreach ($query->getWhereParameterised($parameters) as $where) { - // Check for just the column, in the form '"Column" = ?' and the form '"Table"."Column"' = ? - if (preg_match('/^"([^"]+)"/', $where, $matches) || - preg_match('/^"([^"]+)"\."[^"]+"/', $where, $matches)) { - if (!in_array($matches[1], $queriedColumns)) $queriedColumns[] = $matches[1]; + // Check for any columns in the form '"Column" = ?' or '"Table"."Column"' = ? + if(preg_match_all( + '/(?:"(?[^"]+)"\.)?"(?[^"]+)"(?:[^\.]|$)/', + $where, $matches, PREG_SET_ORDER + )) { + foreach($matches as $match) { + $column = $match['column']; + if (!in_array($column, $queriedColumns)) { + $queriedColumns[] = $column; + } + } } } } else { diff --git a/tests/model/DataQueryTest.php b/tests/model/DataQueryTest.php index d68b5142c..181132e8e 100644 --- a/tests/model/DataQueryTest.php +++ b/tests/model/DataQueryTest.php @@ -224,7 +224,7 @@ class DataQueryTest extends SapphireTest { $query = $query->distinct(true); $this->assertContains('SELECT DISTINCT', $query->sql($params), 'Query contains distinct'); } - + public function testComparisonClauseInt() { DB::query("INSERT INTO \"DataQueryTest_F\" (\"SortOrder\") VALUES (2)"); $query = new DataQuery('DataQueryTest_F'); @@ -232,7 +232,7 @@ class DataQueryTest extends SapphireTest { $this->assertGreaterThan(0, $query->count(), "Couldn't find SortOrder"); $this->resetDBSchema(true); } - + public function testComparisonClauseDateFull() { DB::query("INSERT INTO \"DataQueryTest_F\" (\"MyDate\") VALUES ('1988-03-04 06:30')"); $query = new DataQuery('DataQueryTest_F'); @@ -240,7 +240,7 @@ class DataQueryTest extends SapphireTest { $this->assertGreaterThan(0, $query->count(), "Couldn't find MyDate"); $this->resetDBSchema(true); } - + public function testComparisonClauseDateStartsWith() { DB::query("INSERT INTO \"DataQueryTest_F\" (\"MyDate\") VALUES ('1988-03-04 06:30')"); $query = new DataQuery('DataQueryTest_F'); @@ -248,7 +248,7 @@ class DataQueryTest extends SapphireTest { $this->assertGreaterThan(0, $query->count(), "Couldn't find MyDate"); $this->resetDBSchema(true); } - + public function testComparisonClauseDateStartsPartial() { DB::query("INSERT INTO \"DataQueryTest_F\" (\"MyDate\") VALUES ('1988-03-04 06:30')"); $query = new DataQuery('DataQueryTest_F'); @@ -256,7 +256,7 @@ class DataQueryTest extends SapphireTest { $this->assertGreaterThan(0, $query->count(), "Couldn't find MyDate"); $this->resetDBSchema(true); } - + public function testComparisonClauseTextCaseInsensitive() { DB::query("INSERT INTO \"DataQueryTest_F\" (\"MyString\") VALUES ('HelloWorld')"); $query = new DataQuery('DataQueryTest_F'); @@ -264,19 +264,58 @@ class DataQueryTest extends SapphireTest { $this->assertGreaterThan(0, $query->count(), "Couldn't find MyString"); $this->resetDBSchema(true); } - + public function testComparisonClauseTextCaseSensitive() { DB::query("INSERT INTO \"DataQueryTest_F\" (\"MyString\") VALUES ('HelloWorld')"); $query = new DataQuery('DataQueryTest_F'); $query->where(DB::get_conn()->comparisonClause('"MyString"', 'HelloWorld', false, false, true)); $this->assertGreaterThan(0, $query->count(), "Couldn't find MyString"); - + $query2 = new DataQuery('DataQueryTest_F'); $query2->where(DB::get_conn()->comparisonClause('"MyString"', 'helloworld', false, false, true)); $this->assertEquals(0, $query2->count(), "Found mystring. Shouldn't be able too."); $this->resetDBSchema(true); } + /** + * Tests that getFinalisedQuery can include all tables + */ + public function testConditionsIncludeTables() { + // Including filter on parent table only doesn't pull in second + $query = new DataQuery('DataQueryTest_C'); + $query->sort('"SortOrder"'); + $query->where(array( + '"DataQueryTest_C"."Title" = ?' => array('First') + )); + $result = $query->getFinalisedQuery(array('Title')); + $from = $result->getFrom(); + $this->assertContains('DataQueryTest_C', array_keys($from)); + $this->assertNotContains('DataQueryTest_E', array_keys($from)); + + // Including filter on sub-table requires it + $query = new DataQuery('DataQueryTest_C'); + $query->sort('"SortOrder"'); + $query->where(array( + '"DataQueryTest_C"."Title" = ? OR "DataQueryTest_E"."SortOrder" > ?' => array( + 'First', 2 + ) + )); + $result = $query->getFinalisedQuery(array('Title')); + $from = $result->getFrom(); + + // Check that including "SortOrder" prompted inclusion of DataQueryTest_E table + $this->assertContains('DataQueryTest_C', array_keys($from)); + $this->assertContains('DataQueryTest_E', array_keys($from)); + $arrayResult = iterator_to_array($result->execute()); + $first = array_shift($arrayResult); + $this->assertNotNull($first); + $this->assertEquals('First', $first['Title']); + $second = array_shift($arrayResult); + $this->assertNotNull($second); + $this->assertEquals('Last', $second['Title']); + $this->assertEmpty(array_shift($arrayResult)); + } + }