ENH Allow selecting multiple tables

silverstripe · Sep 21, 2023 · 8921c3f · 8921c3f
1 parent b8665a7
commit 8921c3f
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 18 deletions.
diff --git a/src/ORM/Connect/DBQueryBuilder.php b/src/ORM/Connect/DBQueryBuilder.php
@@ -242,9 +242,25 @@ public function buildUpdateFragment(SQLUpdate $query, array &$parameters)
     public function buildFromFragment(SQLConditionalExpression $query, array &$parameters)
     {
         $from = $query->getJoins($joinParameters);
+        $tables = [];
+        $joins = [];
+
+        // E.g. a naive "Select 1" statemnt is valid SQL
+        if (empty($from)) {
+            return '';
+        }
+
+        foreach ($from as $joinOrTable) {
+            if (preg_match(SQLConditionalExpression::JOIN_REGEX, $joinOrTable)) {
+                $joins[] = $joinOrTable;
+            } else {
+                $tables[] = $joinOrTable;
+            }
+        }
+
         $parameters = array_merge($parameters, $joinParameters);
         $nl = $this->getSeparator();
-        return  "{$nl}FROM " . implode(' ', $from);
+        return  "{$nl}FROM " . implode(', ', $tables) . ' ' . implode(' ', $joins);
     }
 
     /**

diff --git a/src/ORM/Queries/SQLConditionalExpression.php b/src/ORM/Queries/SQLConditionalExpression.php
@@ -2,12 +2,15 @@
 
 namespace SilverStripe\ORM\Queries;
 
+use LogicException;
+
 /**
  * Represents a SQL query for an expression which interacts with existing rows
  * (SELECT / DELETE / UPDATE) with a WHERE clause
  */
 abstract class SQLConditionalExpression extends SQLExpression
 {
+    public const JOIN_REGEX = '/JOIN +.*? +(AS|ON|USING\(?) +/i';
 
     /**
      * An array of WHERE clauses.
@@ -226,7 +229,7 @@ public function queriedTables()
         foreach ($this->from as $key => $tableClause) {
             if (is_array($tableClause)) {
                 $table = '"' . $tableClause['table'] . '"';
-            } elseif (is_string($tableClause) && preg_match('/JOIN +("[^"]+") +(AS|ON) +/i', $tableClause ?? '', $matches)) {
+            } elseif (is_string($tableClause) && preg_match(self::JOIN_REGEX, $tableClause ?? '', $matches)) {
                 $table = $matches[1];
             } else {
                 $table = $tableClause;
@@ -325,11 +328,16 @@ protected function getOrderedJoins($from)
             return $from;
         }
 
-        // shift the first FROM table out from so we only deal with the JOINs
-        reset($from);
-        $baseFromAlias = key($from ?? []);
-        $baseFrom = array_shift($from);
+        // Remove the regular FROM tables out so we only deal with the JOINs
+        $regularTables = [];
+        foreach ($from as $alias => $tableClause) {
+            if (is_string($tableClause) && !preg_match(self::JOIN_REGEX, $tableClause)) {
+                $regularTables[$alias] = $tableClause;
+                unset($from[$alias]);
+            }
+        }
 
+        // Sort the joins
         $this->mergesort($from, function ($firstJoin, $secondJoin) {
             if (!is_array($firstJoin)
                 || !is_array($secondJoin)
@@ -341,11 +349,14 @@ protected function getOrderedJoins($from)
             }
         });
 
-        // Put the first FROM table back into the results
-        if (!empty($baseFromAlias) && !is_numeric($baseFromAlias)) {
-            $from = array_merge([$baseFromAlias => $baseFrom], $from);
-        } else {
-            array_unshift($from, $baseFrom);
+        // Put the regular FROM tables back into the results
+        $regularTables = array_reverse($regularTables, true);
+        foreach ($regularTables as $alias => $tableName) {
+            if (!empty($alias) && !is_numeric($alias)) {
+                $from = array_merge([$alias => $tableName], $from);
+            } else {
+                array_unshift($from, $tableName);
+            }
         }
 
         return $from;

diff --git a/tests/php/ORM/SQLSelectTest.php b/tests/php/ORM/SQLSelectTest.php
@@ -73,13 +73,36 @@ public function testEmptyQueryReturnsNothing()
         $this->assertSQLEquals('', $query->sql($parameters));
     }
 
-    public function testSelectFromBasicTable()
+    public function provideSelectFrom()
+    {
+        return [
+            [
+                'from' => ['MyTable'],
+                'expected' => 'SELECT * FROM MyTable',
+            ],
+            [
+                'from' => ['MyTable', 'MySecondTable'],
+                'expected' => 'SELECT * FROM MyTable, MySecondTable',
+            ],
+            [
+                'from' => ['MyTable', 'INNER JOIN AnotherTable on AnotherTable.ID = MyTable.SomeFieldID'],
+                'expected' => 'SELECT * FROM MyTable INNER JOIN AnotherTable on AnotherTable.ID = MyTable.SomeFieldID',
+            ],
+            [
+                'from' => ['MyTable', 'MySecondTable', 'INNER JOIN AnotherTable on AnotherTable.ID = MyTable.SomeFieldID'],
+                'expected' => 'SELECT * FROM MyTable, MySecondTable INNER JOIN AnotherTable on AnotherTable.ID = MyTable.SomeFieldID',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideSelectFrom
+     */
+    public function testSelectFrom(array $from, string $expected)
     {
         $query = new SQLSelect();
-        $query->setFrom('MyTable');
-        $this->assertSQLEquals("SELECT * FROM MyTable", $query->sql($parameters));
-        $query->addFrom('MyJoin');
-        $this->assertSQLEquals("SELECT * FROM MyTable MyJoin", $query->sql($parameters));
+        $query->setFrom($from);
+        $this->assertSQLEquals($expected, $query->sql($parameters));
     }
 
     public function testSelectFromUserSpecifiedFields()
@@ -819,12 +842,12 @@ public function testBaseTableAliases()
         // In SS4 the "explicitAlias" would be ignored
         $query = SQLSelect::create('*', [
             'MyTableAlias' => '"MyTable"',
-            'explicitAlias' => ', (SELECT * FROM "MyTable" where "something" = "whatever") as "CrossJoin"'
+            'explicitAlias' => '(SELECT * FROM "MyTable" where "something" = "whatever") as "CrossJoin"'
         ]);
         $sql = $query->sql();
 
         $this->assertSQLEquals(
-            'SELECT * FROM "MyTable" AS "MyTableAlias" , ' .
+            'SELECT * FROM "MyTable" AS "MyTableAlias", ' .
             '(SELECT * FROM "MyTable" where "something" = "whatever") as "CrossJoin" AS "explicitAlias"',
             $sql
         );