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..2050fbfe74f 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. @@ -149,6 +152,34 @@ public function addLeftJoin($table, $onPredicate, $tableAlias = '', $order = 20, return $this; } + /** + * Add a RIGHT JOIN criteria to the tables list. + * + * @param string $table Unquoted table name + * @param string $onPredicate The "ON" SQL fragment in a "RIGHT JOIN ... AS ... ON ..." statement, Needs to be valid + * (quoted) SQL. + * @param string $tableAlias Optional alias which makes it easier to identify and replace joins later on + * @param int $order A numerical index to control the order that joins are added to the query; lower order values + * will cause the query to appear first. The default is 20, and joins created automatically by the + * ORM have a value of 10. + * @param array $parameters Any additional parameters if the join is a parameterized subquery + * @return $this Self reference + */ + public function addRightJoin($table, $onPredicate, $tableAlias = '', $order = 20, $parameters = []) + { + if (!$tableAlias) { + $tableAlias = $table; + } + $this->from[$tableAlias] = [ + 'type' => 'RIGHT', + 'table' => $table, + 'filter' => [$onPredicate], + 'order' => $order, + 'parameters' => $parameters + ]; + return $this; + } + /** * Add an INNER JOIN criteria * @@ -226,7 +257,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 +356,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 +377,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/DataQueryTest/DateAndPriceObject.php b/tests/php/ORM/DataQueryTest/DateAndPriceObject.php new file mode 100644 index 00000000000..113aa409619 --- /dev/null +++ b/tests/php/ORM/DataQueryTest/DateAndPriceObject.php @@ -0,0 +1,16 @@ + 'Date', + 'Price' => 'Int', + ]; +} diff --git a/tests/php/ORM/SQLSelectTest/CteDatesObject.php b/tests/php/ORM/SQLSelectTest/CteDatesObject.php new file mode 100644 index 00000000000..d35ea39e531 --- /dev/null +++ b/tests/php/ORM/SQLSelectTest/CteDatesObject.php @@ -0,0 +1,16 @@ + 'Date', + 'Price' => 'Int', + ]; +} diff --git a/tests/php/ORM/SQLSelectTest/CteRecursiveObject.php b/tests/php/ORM/SQLSelectTest/CteRecursiveObject.php new file mode 100644 index 00000000000..6423b2e12d1 --- /dev/null +++ b/tests/php/ORM/SQLSelectTest/CteRecursiveObject.php @@ -0,0 +1,23 @@ + 'Varchar', + ]; + + private static $has_one = [ + 'Parent' => self::class, + ]; + + private static $has_many = [ + 'Children' => self::class . '.Parent', + ]; +}