From 09da83f1cd55343f5162fc1c47a125321dfb696c 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 (or no) tables --- src/ORM/Connect/DBQueryBuilder.php | 18 ++++- src/ORM/Queries/SQLConditionalExpression.php | 37 ++++++--- src/ORM/Queries/SQLSelect.php | 9 +++ tests/php/ORM/SQLSelectTest.php | 82 ++++++++++++++++++-- 4 files changed, 127 insertions(+), 19 deletions(-) diff --git a/src/ORM/Connect/DBQueryBuilder.php b/src/ORM/Connect/DBQueryBuilder.php index c0f1e63f033..32da31fb34f 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" 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 index 55d6ae5c297..ff46eb96c6f 100644 --- 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 index fa38005aaf3..87be4a68384 100644 --- 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 index be70ccb3a93..cce30b721c1 100755 --- 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 );