Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEW Add ORM abstraction for "WITH" clauses #10943

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/ORM/Connect/DBQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace SilverStripe\ORM\Connect;

use InvalidArgumentException;
use SilverStripe\Core\Convert;
use SilverStripe\ORM\Queries\SQLExpression;
use SilverStripe\ORM\Queries\SQLSelect;
use SilverStripe\ORM\Queries\SQLDelete;
Expand Down Expand Up @@ -74,6 +75,7 @@ protected function buildSelectQuery(SQLSelect $query, array &$parameters)
if ($needsParenthisis) {
$sql .= "({$nl}";
}
$sql .= $this->buildWithFragment($query, $parameters);
$sql .= $this->buildSelectFragment($query, $parameters);
$sql .= $this->buildFromFragment($query, $parameters);
$sql .= $this->buildWhereFragment($query, $parameters);
Expand Down Expand Up @@ -165,6 +167,41 @@ protected function buildUpdateQuery(SQLUpdate $query, array &$parameters)
return $sql;
}

/**
* Returns the WITH clauses ready for inserting into a query.
*/
protected function buildWithFragment(SQLSelect $query, array &$parameters): string
{
$with = $query->getWith();
if (empty($with)) {
return '';
}

$nl = $this->getSeparator();
$clauses = [];

foreach ($with as $name => $bits) {
$clause = $bits['recursive'] ? 'RECURSIVE ' : '';
$clause .= Convert::symbol2sql($name);

if (!empty($bits['cte_fields'])) {
$cteFields = $bits['cte_fields'];
// Ensure all cte fields are escaped correctly
array_walk($cteFields, function (&$colName) {
$colName = preg_match('/^".*"$/', $colName) ? $colName : Convert::symbol2sql($colName);
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
});
$clause .= ' (' . implode(', ', $cteFields) . ')';
}

$clause .= " AS ({$nl}";
$clause .= $this->buildSelectQuery($bits['query'], $parameters);
$clause .= "{$nl})";
$clauses[] = $clause;
}

return 'WITH ' . implode(",{$nl}", $clauses) . $nl;
}

/**
* Returns the SELECT clauses ready for inserting into a query.
*
Expand Down
12 changes: 11 additions & 1 deletion src/ORM/Connect/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,17 @@ abstract public function searchEngine(
$invertedMatch = false
);

/**
* Determines if this database supports Common Table Expression (aka WITH) clauses.
* By default it is assumed that it doesn't unless this method is explicitly overridden.
*
* @param bool $recursive if true, checks specifically if recursive CTEs are supported.
*/
public function supportsCteQueries(bool $recursive = false): bool
{
return false;
}

/**
* Determines if this database supports transactions
*
Expand All @@ -654,7 +665,6 @@ public function supportsSavepoints()
return false;
}


/**
* Determines if the used database supports given transactionMode as an argument to startTransaction()
* If transactions are completely unsupported, returns false.
Expand Down
35 changes: 35 additions & 0 deletions src/ORM/Connect/MySQLDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,41 @@ public function searchEngine(
return $list;
}

public function supportsCteQueries(bool $recursive = false): bool
{
$version = $this->getVersion();
$mariaDBVersion = $this->getMariaDBVersion($version);
if ($mariaDBVersion) {
// MariaDB has supported CTEs since 10.2.1, and recursive CTEs from 10.2.2
// see https://mariadb.com/kb/en/mariadb-1021-release-notes/ and https://mariadb.com/kb/en/mariadb-1022-release-notes/
$supportedFrom = $recursive ? '10.2.2' : '10.2.1';
return $this->compareVersion($mariaDBVersion, $supportedFrom) >= 0;
}
// MySQL has supported both kinds of CTEs since 8.0.1
// see https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-1.html
return $this->compareVersion($version, '8.0.1') >= 0;
}

private function getMariaDBVersion(string $version): ?string
{
// MariaDB versions look like "5.5.5-10.6.8-mariadb-1:10.6.8+maria~focal"
// or "10.8.3-MariaDB-1:10.8.3+maria~jammy"
// The relevant part is the x.y.z-mariadb portion.
if (!preg_match('/((\d+\.){2}\d+)-mariadb/i', $version, $matches)) {
return null;
}
return $matches[1];
}

private function compareVersion(string $actualVersion, string $atLeastVersion): int
{
// Assume it's lower if it's not a proper version number
if (!preg_match('/^(\d+\.){2}\d+$/', $actualVersion)) {
return -1;
}
Comment on lines +344 to +347
Copy link
Member Author

@GuySartorelli GuySartorelli Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing this for safety against things like a proxy-db which could have a weird version syntax. I'd much rather just assume those don't support it so we can safely use alternative queries rather than throwing exceptions due to unexpected version strings or incorrectly saying it's supported when it isn't.

return version_compare($actualVersion, $atLeastVersion);
}

/**
* Returns the TransactionManager to handle transactions for this database.
*
Expand Down
37 changes: 34 additions & 3 deletions src/ORM/DataQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ public function having($having)
*/
public function union(DataQuery|SQLSelect $query, ?string $type = null): static
{
if ($query instanceof self) {
if ($query instanceof DataQuery) {
$query = $query->query();
}
$this->query->addUnion($query, $type);
Expand All @@ -701,8 +701,6 @@ public function disjunctiveGroup()
return new DataQuery_SubGroup($this, 'OR', $clause);
}



/**
* Create a conjunctive subgroup
*
Expand All @@ -723,6 +721,39 @@ public function conjunctiveGroup()
return new DataQuery_SubGroup($this, 'AND', $clause);
}

/**
* Adds a Common Table Expression (CTE), aka WITH clause.
*
* Use of this method should usually be within a conditional check against DB::get_conn()->supportsCteQueries().
*
* @param string $name The name of the WITH clause, which can be referenced in any queries UNIONed to the $query
* and in this query directly, as though it were a table name.
* @param string[] $cteFields Aliases for any columns selected in $query which can be referenced in any queries
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
* UNIONed to the $query and in this query directly, as though they were columns in a real table.
* NOTE: If $query is a DataQuery, then cteFields must be the names of real columns on that DataQuery's data class.
*/
public function with(string $name, DataQuery|SQLSelect $query, array $cteFields = [], bool $recursive = false): static
{
$schema = DataObject::getSchema();

// If the query is a DataQuery, make sure all manipulators, joins, etc are applied
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
if ($query instanceof self) {
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
$cteDataClass = $query->dataClass();
$query = $query->query();
// DataQuery wants to select ALL columns by default,
// but if we're setting cteFields then we only want to select those fields.
if (!empty($cteFields)) {
$selectFields = array_map(fn($colName) => $schema->sqlColumnForField($cteDataClass, $colName), $cteFields);
$query->setSelect($selectFields);
}
}

// Add the WITH clause
$this->query->addWith($name, $query, $cteFields, $recursive);

return $this;
}

/**
* Adds a WHERE clause.
*
Expand Down
45 changes: 44 additions & 1 deletion src/ORM/Queries/SQLSelect.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@ class SQLSelect extends SQLConditionalExpression
*/
protected array $union = [];

/**
* An array of WITH clauses.
* This array is indexed with the name for the temporary table generated for the WITH clause,
* and contains data in the following format:
* [
* 'cte_fields' => string[],
* 'query' => SQLSelect|null,
* 'recursive' => boolean,
* ]
*/
protected array $with = [];

/**
* If this is true DISTINCT will be added to the SQL.
*
Expand Down Expand Up @@ -546,7 +558,7 @@ public function getHavingParameterised(&$parameters)
*
* @param string|null $type One of the UNION_ALL or UNION_DISTINCT constants - or null for a default union
*/
public function addUnion(self $query, ?string $type = null): static
public function addUnion(SQLSelect $query, ?string $type = null): static
{
if ($type && $type !== self::UNION_ALL && $type !== self::UNION_DISTINCT) {
throw new LogicException('Union $type must be one of the constants UNION_ALL or UNION_DISTINCT.');
Expand All @@ -564,6 +576,37 @@ public function getUnions(): array
return $this->union;
}

/**
* Adds a Common Table Expression (CTE), aka WITH clause.
*
* Use of this method should usually be within a conditional check against DB::get_conn()->supportsCteQueries().
*
* @param string $name The name of the WITH clause, which can be referenced in any queries UNIONed to the $query
* and in this query directly, as though it were a table name.
* @param string[] $cteFields Aliases for any columns selected in $query which can be referenced in any queries
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
* UNIONed to the $query and in this query directly, as though they were columns in a real table.
*/
public function addWith(string $name, SQLSelect $query, array $cteFields = [], bool $recursive = false): static
{
if (array_key_exists($name, $this->with)) {
throw new LogicException("WITH clause with name '$name' already exists.");
}
$this->with[$name] = [
'cte_fields' => $cteFields,
'query' => $query,
'recursive' => $recursive,
];
return $this;
}

/**
* Get the data which will be used to generate the WITH clause of the query
*/
public function getWith(): array
{
return $this->with;
}

/**
* Return a list of GROUP BY clauses used internally.
*
Expand Down
Loading