From b34c236b3c26ff61f58d6389866ad3a741ef37d3 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Wed, 18 Mar 2015 16:09:09 +1300
Subject: [PATCH] BUG Fix joins on tables containing "select" being mistaken
 for sub-selects Fix PHPDoc on SQLQuery::addFrom and SQLQuery::setFrom Fixes
 #3965

---
 model/SQLQuery.php           | 22 +++++++++++++---------
 tests/model/SQLQueryTest.php | 35 +++++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/model/SQLQuery.php b/model/SQLQuery.php
index 2965d062b..c1c0e5b3f 100644
--- a/model/SQLQuery.php
+++ b/model/SQLQuery.php
@@ -239,10 +239,10 @@ class SQLQuery {
 	/**
 	 * Set table for the SELECT clause.
 	 *
-	 * @example $query->setFrom("MyTable"); // SELECT * FROM MyTable
+	 * @example $query->setFrom('"MyTable"'); // SELECT * FROM "MyTable"
 	 *
-	 * @param string|array $from Escaped SQL statement, usually an unquoted table name
-	 * @return SQLQuery
+	 * @param string|array $from Single, or list of, ANSI quoted table names
+	 * @return self
 	 */
 	public function setFrom($from) {
 		$this->from = array();
@@ -252,10 +252,10 @@ class SQLQuery {
 	/**
 	 * Add a table to the SELECT clause.
 	 *
-	 * @example $query->addFrom("MyTable"); // SELECT * FROM MyTable
+	 * @example $query->addFrom('"MyTable"'); // SELECT * FROM "MyTable"
 	 *
-	 * @param string|array $from Escaped SQL statement, usually an unquoted table name
-	 * @return SQLQuery
+	 * @param string|array $from Single, or list of, ANSI quoted table names
+	 * @return self Self reference
 	 */
 	public function addFrom($from) {
 		if(is_array($from)) {
@@ -903,9 +903,13 @@ class SQLQuery {
 				else if(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 \"" . Convert::raw2sql($alias) . "\"" : "";
+				// Ensure tables are quoted, unless the table is actually a sub-select
+				$table = preg_match('/\bSELECT\b/i', $join['table'])
+					? $join['table']
+					: "\"{$join['table']}\"";
+				$aliasClause = ($alias != $join['table'])
+						? " AS \"{$alias}\""
+						: "";
 				$this->from[$alias] = strtoupper($join['type']) . " JOIN " 
 					. $table . "$aliasClause ON $filter";
 			}
diff --git a/tests/model/SQLQueryTest.php b/tests/model/SQLQueryTest.php
index 645d52cf3..12cf229f7 100755
--- a/tests/model/SQLQueryTest.php
+++ b/tests/model/SQLQueryTest.php
@@ -341,26 +341,37 @@ class SQLQueryTest extends SapphireTest {
 			$query->sql()
 		);
 	}
-	
+
 	public function testJoinSubSelect() {
 
+		// Test sub-select works
 		$query = new SQLQuery();
-		$query->setFrom('MyTable');
-		$query->addInnerJoin('(SELECT * FROM MyOtherTable)', 
-			'Mot.MyTableID = MyTable.ID', 'Mot');
-		$query->addLeftJoin('(SELECT MyLastTable.MyOtherTableID, COUNT(1) as MyLastTableCount FROM MyLastTable '
-			. 'GROUP BY MyOtherTableID)', 
-			'Mlt.MyOtherTableID = Mot.ID', 'Mlt');
-		$query->setOrderBy('COALESCE(Mlt.MyLastTableCount, 0) DESC');
+		$query->setFrom('"MyTable"');
+		$query->addInnerJoin('(SELECT * FROM "MyOtherTable")',
+			'"Mot"."MyTableID" = "MyTable"."ID"', 'Mot');
+		$query->addLeftJoin('(SELECT "MyLastTable"."MyOtherTableID", COUNT(1) as "MyLastTableCount" '
+			. 'FROM "MyLastTable" GROUP BY "MyOtherTableID")',
+			'"Mlt"."MyOtherTableID" = "Mot"."ID"', 'Mlt');
+		$query->setOrderBy('COALESCE("Mlt"."MyLastTableCount", 0) DESC');
 
-		$this->assertEquals('SELECT *, COALESCE(Mlt.MyLastTableCount, 0) AS "_SortColumn0" FROM MyTable '.
-			'INNER JOIN (SELECT * FROM MyOtherTable) AS "Mot" ON Mot.MyTableID = MyTable.ID ' .
-			'LEFT JOIN (SELECT MyLastTable.MyOtherTableID, COUNT(1) as MyLastTableCount FROM MyLastTable '
-			. 'GROUP BY MyOtherTableID) AS "Mlt" ON Mlt.MyOtherTableID = Mot.ID ' .
+		$this->assertEquals('SELECT *, COALESCE("Mlt"."MyLastTableCount", 0) AS "_SortColumn0" FROM "MyTable" '.
+			'INNER JOIN (SELECT * FROM "MyOtherTable") AS "Mot" ON "Mot"."MyTableID" = "MyTable"."ID" ' .
+			'LEFT JOIN (SELECT "MyLastTable"."MyOtherTableID", COUNT(1) as "MyLastTableCount" FROM "MyLastTable" '
+			. 'GROUP BY "MyOtherTableID") AS "Mlt" ON "Mlt"."MyOtherTableID" = "Mot"."ID" ' .
 			'ORDER BY "_SortColumn0" DESC',
 			$query->sql()
 		);
 
+		// Test that table names do not get mistakenly identified as sub-selects
+		$query = new SQLQuery();
+		$query->setFrom('"MyTable"');
+		$query->addInnerJoin('NewsArticleSelected', '"News"."MyTableID" = "MyTable"."ID"', 'News');
+		$this->assertEquals(
+			'SELECT * FROM "MyTable" INNER JOIN "NewsArticleSelected" AS "News" ON '.
+			'"News"."MyTableID" = "MyTable"."ID"',
+			$query->sql()
+		);
+
 	}
 	
 	public function testSetWhereAny() {