From 8921c3f011e4f5d194e492293014d97ea7094674 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 4 Aug 2023 15:38:28 +1200 Subject: [PATCH] ENH Allow selecting multiple tables --- src/ORM/Connect/DBQueryBuilder.php | 18 +++++++++- src/ORM/Queries/SQLConditionalExpression.php | 31 ++++++++++------ tests/php/ORM/SQLSelectTest.php | 37 ++++++++++++++++---- 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/src/ORM/Connect/DBQueryBuilder.php b/src/ORM/Connect/DBQueryBuilder.php index c0f1e63f033..003dee9d80c 100644 --- 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 index 55d6ae5c297..c694308f446 100644 --- 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 index be70ccb3a93..25358c7b82e 100755 --- 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 );