ENH Allow selecting multiple (or no) tables (#10953)

silverstripe · Sep 24, 2023 · b28749d · b28749d
1 parent b3b1d07
commit b28749d
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 19 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" statement is valid SQL
+        if (empty($from)) {
+            return '';
+        }
+
+        foreach ($from as $joinOrTable) {
+            if (preg_match(SQLConditionalExpression::getJoinRegex(), $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
@@ -8,7 +8,6 @@
  */
 abstract class SQLConditionalExpression extends SQLExpression
 {
-
     /**
      * An array of WHERE clauses.
      *
@@ -226,7 +225,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::getJoinRegex(), $tableClause ?? '', $matches)) {
                 $table = $matches[1];
             } else {
                 $table = $tableClause;
@@ -325,11 +324,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::getJoinRegex(), $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 +345,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;
@@ -773,4 +780,12 @@ public function toUpdate()
         $this->copyTo($update);
         return $update;
     }
+
+    /**
+     * Get the regular expression pattern used to identify JOIN statements
+     */
+    public static function getJoinRegex(): string
+    {
+        return '/JOIN +.*? +(AS|ON|USING\(?) +/i';
+    }
 }
diff --git a/src/ORM/Queries/SQLSelect.php b/src/ORM/Queries/SQLSelect.php
@@ -162,6 +162,9 @@ public function addSelect($fields)
             $fields = [$fields];
         }
         foreach ($fields as $idx => $field) {
+            if ($field === '') {
+                continue;
+            }
             $this->selectField($field, is_numeric($idx) ? null : $idx);
         }
 
@@ -713,4 +716,10 @@ public function lastRow()
         $query->setLimit(1, $index);
         return $query;
     }
+
+    public function isEmpty()
+    {
+        // Empty if there's no select, or we're trying to select '*' but there's no FROM clause
+        return empty($this->select) || (empty($this->from) && array_key_exists('*', $this->select));
+    }
 }
diff --git a/tests/php/ORM/SQLSelectTest.php b/tests/php/ORM/SQLSelectTest.php
@@ -67,19 +67,80 @@ public function testUnlimitedRowCount()
         }
     }
 
+    public function provideIsEmpty()
+    {
+        return [
+            [
+                'query' => new SQLSelect(),
+                'expected' => true,
+            ],
+            [
+                'query' => new SQLSelect(from: 'someTable'),
+                'expected' => false,
+            ],
+            [
+                'query' => new SQLSelect(''),
+                'expected' => true,
+            ],
+            [
+                'query' => new SQLSelect('', 'someTable'),
+                'expected' => true,
+            ],
+            [
+                'query' => new SQLSelect('column', 'someTable'),
+                'expected' => false,
+            ],
+            [
+                'query' => new SQLSelect('value'),
+                'expected' => false,
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideIsEmpty
+     */
+    public function testIsEmpty(SQLSelect $query, $expected)
+    {
+        $this->assertSame($expected, $query->isEmpty());
+    }
+
     public function testEmptyQueryReturnsNothing()
     {
         $query = new SQLSelect();
         $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()
@@ -724,6 +785,13 @@ public function testSelect()
         );
     }
 
+    public function testSelectWithNoTable()
+    {
+        $query = new SQLSelect('200');
+        $this->assertSQLEquals('SELECT 200 AS "200"', $query->sql());
+        $this->assertSame([['200' => 200]], iterator_to_array($query->execute(), true));
+    }
+
     /**
      * Test passing in a LIMIT with OFFSET clause string.
      */
@@ -819,12 +887,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
         );