Merge pull request #3490 from tractorcow/pulls/3.2/parameterised-joins

API Allow parameterised joins / subselects

docs/en/changelogs/3.2.0.md

@@ -305,10 +305,10 @@ Examples of areas where queries should be upgraded are below:
 			'"Title" = ?', $title
 		);
 
-4. #### Interaction with the `DataList::sql()`, `DataQuery::sql()` or `SQLSelect::sql()` methods
+4. #### Interaction with `DataList::sql()`, `DataQuery::sql()`, `SQLSelect::sql()`, or `SQLSelect::getJoins()` methods
 
 	The place where legacy code would almost certainly fail is any code that calls
-	`SQLQuery::sql`, `DataList::sql` or `DataQuery::sql`, as the api requires that user
+	`SQLQuery::sql`, `DataList::sql`, `DataQuery::sql` or `SQLSelect::getJoins()`, as the api requires that user
 	code passes in an argument here to retrieve SQL parameters by value.
 
 	User code that assumes parameterless queries will likely fail, and need to be

docs/en/topics/datamodel.md

@@ -485,6 +485,8 @@ methods have the same arguments:
  * The name of the table to join to
  * The filter clause for the join
  * An optional alias
+ * Priority (to allow you to later sort joins)
+ * An optional list of parameters (in case you wish to use a parameterised subselect).
 
 For example:
 
@@ -495,6 +497,17 @@ For example:
 
 	$members = Member::get()
 		->innerJoin("Group_Members", "\"Rel\".\"MemberID\" = \"Member\".\"ID\"", "Rel");
+
+	// With a subselect
+	$members = Member::get()
+		->innerJoin(
+			'(SELECT "MemberID", COUNT("ID") AS "Count" FROM "Member_Likes" GROUP BY "MemberID" HAVING "Count" >= ?)',
+			'"Likes"."MemberID" = "Member"."ID"',
+			"Likes",
+			20,
+			array($threshold)
+		);
+
 	
 Passing a *$join* statement to DataObject::get will filter results further by
 the JOINs performed against the foreign table. **It will NOT return the

model/DataList.php

@@ -599,11 +599,15 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @param string $table Table name (unquoted and as escaped SQL)
 	 * @param string $onClause Escaped SQL statement, e.g. '"Table1"."ID" = "Table2"."ID"'
 	 * @param string $alias - if you want this table to be aliased under another name
+	 * @param int $order A numerical index to control the order that joins are added to the query; lower order values
+	 * will cause the query to appear first. The default is 20, and joins created automatically by the
+	 * ORM have a value of 10.
+	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
 	 * @return DataList
 	 */
-	public function innerJoin($table, $onClause, $alias = null) {
+	public function innerJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
-		return $this->alterDataQuery(function($query) use ($table, $onClause, $alias){
+		return $this->alterDataQuery(function($query) use ($table, $onClause, $alias, $order, $parameters){
-			$query->innerJoin($table, $onClause, $alias);
+			$query->innerJoin($table, $onClause, $alias, $order, $parameters);
 		});
 	}
 
@@ -613,11 +617,15 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @param string $table Table name (unquoted and as escaped SQL)
 	 * @param string $onClause Escaped SQL statement, e.g. '"Table1"."ID" = "Table2"."ID"'
 	 * @param string $alias - if you want this table to be aliased under another name
+	 * @param int $order A numerical index to control the order that joins are added to the query; lower order values
+	 * will cause the query to appear first. The default is 20, and joins created automatically by the
+	 * ORM have a value of 10.
+	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
 	 * @return DataList
 	 */
-	public function leftJoin($table, $onClause, $alias = null) {
+	public function leftJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
-		return $this->alterDataQuery(function($query) use ($table, $onClause, $alias){
+		return $this->alterDataQuery(function($query) use ($table, $onClause, $alias, $order, $parameters){
-			$query->leftJoin($table, $onClause, $alias);
+			$query->leftJoin($table, $onClause, $alias, $order, $parameters);
 		});
 	}
 

model/DataQuery.php

@@ -583,10 +583,14 @@ class DataQuery {
 	 * @param String $table The unquoted table name to join to.
 	 * @param String $onClause The filter for the join (escaped SQL statement)
 	 * @param String $alias An optional alias name (unquoted)
+	 * @param int $order A numerical index to control the order that joins are added to the query; lower order values
+	 * will cause the query to appear first. The default is 20, and joins created automatically by the
+	 * ORM have a value of 10.
+	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
 	 */
-	public function innerJoin($table, $onClause, $alias = null) {
+	public function innerJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
 		if($table) {
-			$this->query->addInnerJoin($table, $onClause, $alias);
+			$this->query->addInnerJoin($table, $onClause, $alias, $order, $parameters);
 		}
 		return $this;
 	}
@@ -594,13 +598,17 @@ class DataQuery {
 	/**
 	 * Add a LEFT JOIN clause to this query.
 	 *
-	 * @param String $table The unquoted table to join to.
+	 * @param string $table The unquoted table to join to.
-	 * @param String $onClause The filter for the join (escaped SQL statement).
+	 * @param string $onClause The filter for the join (escaped SQL statement).
-	 * @param String $alias An optional alias name (unquoted)
+	 * @param string $alias An optional alias name (unquoted)
+	 * @param int $order A numerical index to control the order that joins are added to the query; lower order values
+	 * will cause the query to appear first. The default is 20, and joins created automatically by the
+	 * ORM have a value of 10.
+	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
 	 */
-	public function leftJoin($table, $onClause, $alias = null) {
+	public function leftJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
 		if($table) {
-			$this->query->addLeftJoin($table, $onClause, $alias);
+			$this->query->addLeftJoin($table, $onClause, $alias, $order, $parameters);
 		}
 		return $this;
 	}

model/connect/DBQueryBuilder.php

@@ -216,8 +216,9 @@ class DBQueryBuilder {
 	 * @param array $parameters Out parameter for the resulting query parameters
 	 * @return string Completed from part of statement
 	 */
-	public function buildFromFragment(SQLExpression $query, array &$parameters) {
+	public function buildFromFragment(SQLConditionalExpression $query, array &$parameters) {
-		$from = $query->getJoins();
+		$from = $query->getJoins($joinParameters);
+		$parameters = array_merge($parameters, $joinParameters);
 		$nl = $this->getSeparator();
 		return  "{$nl}FROM " . implode(' ', $from);
 	}
@@ -229,8 +230,7 @@ class DBQueryBuilder {
 	 * @param array $parameters Out parameter for the resulting query parameters
 	 * @return string Completed where condition
 	 */
-	public function buildWhereFragment(SQLExpression $query, array &$parameters) {
+	public function buildWhereFragment(SQLConditionalExpression $query, array &$parameters) {
-
 		// Get parameterised elements
 		$where = $query->getWhereParameterised($whereParameters);
 		if(empty($where)) return '';

model/queries/SQLConditionalExpression.php

@@ -124,9 +124,10 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	 * @param int $order A numerical index to control the order that joins are added to the query; lower order values
 	 *                   will cause the query to appear first. The default is 20, and joins created automatically by the
 	 *                   ORM have a value of 10.
+	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
 	 * @return self Self reference
 	 */
-	public function addLeftJoin($table, $onPredicate, $tableAlias = '', $order = 20) {
+	public function addLeftJoin($table, $onPredicate, $tableAlias = '', $order = 20, $parameters = array()) {
 		if(!$tableAlias) {
 			$tableAlias = $table;
 		}
@@ -134,7 +135,8 @@ abstract class SQLConditionalExpression extends SQLExpression {
 			'type' => 'LEFT',
 			'table' => $table,
 			'filter' => array($onPredicate),
-			'order' => $order
+			'order' => $order,
+			'parameters' => $parameters
 		);
 		return $this;
 	}
@@ -149,15 +151,17 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	 * @param int $order A numerical index to control the order that joins are added to the query; lower order
 	 * values will cause the query to appear first. The default is 20, and joins created automatically by the
 	 * ORM have a value of 10.
+	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
 	 * @return self Self reference
 	 */
-	public function addInnerJoin($table, $onPredicate, $tableAlias = null, $order = 20) {
+	public function addInnerJoin($table, $onPredicate, $tableAlias = null, $order = 20, $parameters = array()) {
 		if(!$tableAlias) $tableAlias = $table;
 		$this->from[$tableAlias] = array(
 			'type' => 'INNER',
 			'table' => $table,
 			'filter' => array($onPredicate),
-			'order' => $order
+			'order' => $order,
+			'parameters' => $parameters
 		);
 		return $this;
 	}
@@ -236,10 +240,20 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	 *
 	 * @todo This part of the code could be simplified
 	 *
+	 * @param array $parameters Out variable for parameters required for this query
 	 * @return array List of joins as a mapping from array('Alias' => 'Join Expression')
 	 */
-	public function getJoins() {
+	public function getJoins(&$parameters = array()) {
+		if(func_num_args() == 0) {
+			Deprecation::notice(
+				'3.2',
+				'SQLConditionalExpression::getJoins() now may produce parameters which are necessary to
+				execute this query'
+			);
+		}
+		
 		// Sort the joins
+		$parameters = array();
 		$joins = $this->getOrderedJoins($this->from);
 
 		// Build from clauses
@@ -247,18 +261,21 @@ abstract class SQLConditionalExpression extends SQLExpression {
 			// $join can be something like this array structure
 			// array('type' => 'inner', 'table' => 'SiteTree', 'filter' => array("SiteTree.ID = 1",
 			// "Status = 'approved'", 'order' => 20))
-			if(is_array($join)) {
+			if(!is_array($join)) continue;
-				if(is_string($join['filter'])) {
-					$filter = $join['filter'];
-				} elseif(sizeof($join['filter']) == 1) {
-					$filter = $join['filter'][0];
-				} else {
-					$filter = "(" . implode(") AND (", $join['filter']) . ")";
-				}
 
-				$table = strpos(strtoupper($join['table']), 'SELECT') ? $join['table'] : "\"" . $join['table'] . "\"";
+			if(is_string($join['filter'])) {
-				$aliasClause = ($alias != $join['table']) ? " AS \"$alias\"" : "";
+				$filter = $join['filter'];
-				$joins[$alias] = strtoupper($join['type']) . " JOIN " . $table . "$aliasClause ON $filter";
+			} elseif(sizeof($join['filter']) == 1) {
+				$filter = $join['filter'][0];
+			} else {
+				$filter = "(" . implode(") AND (", $join['filter']) . ")";
+			}
+
+			$table = strpos(strtoupper($join['table']), 'SELECT') ? $join['table'] : "\"" . $join['table'] . "\"";
+			$aliasClause = ($alias != $join['table']) ? " AS \"$alias\"" : "";
+			$joins[$alias] = strtoupper($join['type']) . " JOIN " . $table . "$aliasClause ON $filter";
+			if(!empty($join['parameters'])) {
+				$parameters = array_merge($parameters, $join['parameters']);
 			}
 		}
 

tests/model/DataListTest.php

@@ -168,6 +168,36 @@ class DataListTest extends SapphireTest {
 			. '"DataObjectTest_TeamComment"."TeamID"';
 
 		$this->assertSQLEquals($expected, $list->sql($parameters));
+		$this->assertEmpty($parameters);
+	}
+
+	public function testInnerJoinParameterised() {
+		$db = DB::get_conn();
+		
+		$list = DataObjectTest_TeamComment::get();
+		$list = $list->innerJoin(
+			'DataObjectTest_Team',
+			'"DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID" '
+			. 'AND "DataObjectTest_Team"."Title" LIKE ?',
+			'Team',
+			20,
+			array('Team%')
+		);
+
+		$expected = 'SELECT DISTINCT "DataObjectTest_TeamComment"."ClassName", '
+			. '"DataObjectTest_TeamComment"."LastEdited", "DataObjectTest_TeamComment"."Created", '
+			. '"DataObjectTest_TeamComment"."Name", "DataObjectTest_TeamComment"."Comment", '
+			. '"DataObjectTest_TeamComment"."TeamID", "DataObjectTest_TeamComment"."ID", '
+			. 'CASE WHEN "DataObjectTest_TeamComment"."ClassName" IS NOT NULL'
+			. ' THEN "DataObjectTest_TeamComment"."ClassName" ELSE '
+			. $db->quoteString('DataObjectTest_TeamComment')
+			. ' END AS "RecordClassName" FROM "DataObjectTest_TeamComment" INNER JOIN '
+			. '"DataObjectTest_Team" AS "Team" ON "DataObjectTest_Team"."ID" = '
+			. '"DataObjectTest_TeamComment"."TeamID" '
+			. 'AND "DataObjectTest_Team"."Title" LIKE ?';
+
+		$this->assertSQLEquals($expected, $list->sql($parameters));
+		$this->assertEquals(array('Team%'), $parameters);
 	}
 
 	public function testLeftJoin() {
@@ -191,6 +221,7 @@ class DataListTest extends SapphireTest {
 			. 'AS "Team" ON "DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID"';
 
 		$this->assertSQLEquals($expected, $list->sql($parameters));
+		$this->assertEmpty($parameters);
 
 		// Test with namespaces (with non-sensical join, but good enough for testing)
 		$list = DataObjectTest_TeamComment::get();
@@ -213,9 +244,39 @@ class DataListTest extends SapphireTest {
 			. 'LEFT JOIN "DataObjectTest\NamespacedClass" ON '
 			. '"DataObjectTest\NamespacedClass"."ID" = "DataObjectTest_TeamComment"."ID"';
 		$this->assertSQLEquals($expected, $list->sql($parameters), 'Retains backslashes in namespaced classes');
+		$this->assertEmpty($parameters);
 
 	}
 
+	public function testLeftJoinParameterised() {
+		$db = DB::get_conn();
+
+		$list = DataObjectTest_TeamComment::get();
+		$list = $list->leftJoin(
+			'DataObjectTest_Team',
+			'"DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID" '
+			. 'AND "DataObjectTest_Team"."Title" LIKE ?',
+			'Team',
+			20,
+			array('Team%')
+		);
+
+		$expected = 'SELECT DISTINCT "DataObjectTest_TeamComment"."ClassName", '
+			. '"DataObjectTest_TeamComment"."LastEdited", "DataObjectTest_TeamComment"."Created", '
+			. '"DataObjectTest_TeamComment"."Name", "DataObjectTest_TeamComment"."Comment", '
+			. '"DataObjectTest_TeamComment"."TeamID", "DataObjectTest_TeamComment"."ID", '
+			. 'CASE WHEN "DataObjectTest_TeamComment"."ClassName" IS NOT NULL'
+			. ' THEN "DataObjectTest_TeamComment"."ClassName" ELSE '
+			. $db->quoteString('DataObjectTest_TeamComment')
+			. ' END AS "RecordClassName" FROM "DataObjectTest_TeamComment" LEFT JOIN '
+			. '"DataObjectTest_Team" AS "Team" ON "DataObjectTest_Team"."ID" = '
+			. '"DataObjectTest_TeamComment"."TeamID" '
+			. 'AND "DataObjectTest_Team"."Title" LIKE ?';
+
+		$this->assertSQLEquals($expected, $list->sql($parameters));
+		$this->assertEquals(array('Team%'), $parameters);
+	}
+
 	public function testToNestedArray() {
 		$list = DataObjectTest_TeamComment::get()->sort('ID');
 		$nestedArray = $list->toNestedArray();

tests/model/SQLQueryTest.php

@@ -10,6 +10,8 @@ class SQLQueryTest extends SapphireTest {
 
 	protected $extraDataObjects = array(
 		'SQLQueryTest_DO',
+		'SQLQueryTestBase',
+		'SQLQueryTestChild'
 	);
 
 	public function testEmptyQueryReturnsNothing() {
@@ -579,6 +581,51 @@ class SQLQueryTest extends SapphireTest {
 		$this->assertEquals(10, $limit['start']);
 	}
 
+	public function testParameterisedInnerJoins() {
+		$query = new SQLSelect();
+		$query->setSelect(array('"SQLQueryTest_DO"."Name"', '"SubSelect"."Count"'));
+		$query->setFrom('"SQLQueryTest_DO"');
+		$query->addInnerJoin(
+			'(SELECT "Title", COUNT(*) AS "Count" FROM "SQLQueryTestBase" GROUP BY "Title" HAVING "Title" NOT LIKE ?)',
+			'"SQLQueryTest_DO"."Name" = "SubSelect"."Title"',
+			'SubSelect',
+			20,
+			array('%MyName%')
+		);
+		$query->addWhere(array('"SQLQueryTest_DO"."Date" > ?' => '2012-08-08 12:00'));
+
+		$this->assertSQLEquals('SELECT "SQLQueryTest_DO"."Name", "SubSelect"."Count"
+			FROM "SQLQueryTest_DO" INNER JOIN (SELECT "Title", COUNT(*) AS "Count" FROM "SQLQueryTestBase"
+		   GROUP BY "Title" HAVING "Title" NOT LIKE ?) AS "SubSelect" ON "SQLQueryTest_DO"."Name" =
+		   "SubSelect"."Title"
+			WHERE ("SQLQueryTest_DO"."Date" > ?)', $query->sql($parameters)
+		);
+		$this->assertEquals(array('%MyName%', '2012-08-08 12:00'), $parameters);
+		$query->execute();
+	}
+
+	public function testParameterisedLeftJoins() {
+		$query = new SQLSelect();
+		$query->setSelect(array('"SQLQueryTest_DO"."Name"', '"SubSelect"."Count"'));
+		$query->setFrom('"SQLQueryTest_DO"');
+		$query->addLeftJoin(
+			'(SELECT "Title", COUNT(*) AS "Count" FROM "SQLQueryTestBase" GROUP BY "Title" HAVING "Title" NOT LIKE ?)',
+			'"SQLQueryTest_DO"."Name" = "SubSelect"."Title"',
+			'SubSelect',
+			20,
+			array('%MyName%')
+		);
+		$query->addWhere(array('"SQLQueryTest_DO"."Date" > ?' => '2012-08-08 12:00'));
+
+		$this->assertSQLEquals('SELECT "SQLQueryTest_DO"."Name", "SubSelect"."Count"
+			FROM "SQLQueryTest_DO" LEFT JOIN (SELECT "Title", COUNT(*) AS "Count" FROM "SQLQueryTestBase"
+		   GROUP BY "Title" HAVING "Title" NOT LIKE ?) AS "SubSelect" ON "SQLQueryTest_DO"."Name" =
+		   "SubSelect"."Title"
+			WHERE ("SQLQueryTest_DO"."Date" > ?)', $query->sql($parameters)
+		);
+		$this->assertEquals(array('%MyName%', '2012-08-08 12:00'), $parameters);
+		$query->execute();
+	}
 }
 
 class SQLQueryTest_DO extends DataObject implements TestOnly {