-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add CTE support to select in QueryBuilder #6621
base: 4.3.x
Are you sure you want to change the base?
Changes from 3 commits
6559c32
11280cd
19b6ad3
aab2338
3fdc8d8
b20f5e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Doctrine\DBAL\Query; | ||
|
||
use function count; | ||
use function sprintf; | ||
|
||
/** @internal */ | ||
final class CommonTableExpression | ||
{ | ||
/** @param string[]|null $columns */ | ||
public function __construct( | ||
public readonly string $name, | ||
public readonly string|QueryBuilder $query, | ||
public readonly ?array $columns, | ||
) { | ||
if ($columns !== null && count($columns) === 0) { | ||
throw new QueryException(sprintf('Columns defined in CTE "%s" should not be an empty array.', $name)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -160,6 +160,13 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private array $unionParts = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* The common table expression parts. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @var CommonTableExpression[] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private array $commonTableExpressions = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* The query cache profile used for caching results. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -302,14 +309,47 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public function executeQuery(): Result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[$params, $types] = $this->buildParametersAndTypes(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return $this->connection->executeQuery( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$this->getSQL(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$this->params, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$this->types, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$params, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$types, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$this->resultCacheProfile, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Build then return parameters and types for the query. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @return array{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* list<mixed>|array<string, mixed>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* WrapperParameterTypeArray, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* } The parameters and types for the query. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private function buildParametersAndTypes(): array | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$cteParams = $cteParamTypes = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
foreach ($this->commonTableExpressions as $cte) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (! $cte->query instanceof self) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$cteParams = array_merge($cteParams, $cte->query->params); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This overrides named placehoders, if two part QueryBuilder instances auto creates named placeholders, specially when $cteQueryBuilder1 = $this->connection->createQueryBuilder();
$cteQueryBuilder1->select('id AS virtual_id')
->from('for_update')
->where('virtual_id = ' . $cteQueryBuilder1->createNamedParameter(123, ParameterType::INTEGER));
$cteQueryBuilder2 = $this->connection->createQueryBuilder();
$cteQueryBuilder2->select('id AS virtual_id')
->from('cte_a')
->where('virtual_id = ' . $cteQueryBuilder2->createNamedParameter(987, ParameterType::INTEGER)); would merge them in a weired way. Additionally, the tests related to named placeholder only covers manual defining them with setting parameter, not using the create methods along with autogeneration and using in two sub-querybuilder instances which gets merged. dbal/src/Query/QueryBuilder.php Lines 1403 to 1440 in c91166a
Tests related to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That does not work with default (auto) placeholder because of the ->where($cteQueryBuilder1->expr()->eq(
'id',
$cteQueryBuilder1->createNamedParameter(1, ParameterType::INTEGER, ':id1'),
));
->where($cteQueryBuilder2->expr()->in(
'id',
$cteQueryBuilder2->createNamedParameter([1, 2], ArrayParameterType::INTEGER, ':id2'),
)); I've added some tests for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My whole point is, that starting this leads to more things some developers needs to know beside a simple "use the top QueryBuilder" instance. Not a fan of starting this and leave it half way broken and unfinished. But that is more a personal taste:
In that case database error should occor and be a little bit of help that something is off when merging the CTE parts on query execution.
would merge that and use Askin me the whole merging should vanish again until it could be implemented completly in all parts and with a concept behind it. But that is not my decision. Let's see what @derrabus would say. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, it could give a hard time to debug this. We could prevent this by adding a check during the merge process and reject a parameter's name that is already used. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$cteParamTypes = array_merge($cteParamTypes, $cte->query->types); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (count($cteParams) === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return [$this->params, $this->types]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
array_merge($cteParams, $this->params), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
array_merge($cteParamTypes, $this->types), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Executes an SQL statement and returns the number of affected rows. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -557,6 +597,36 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return $this; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Add a Common Table Expression to be used for a select query. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* <code> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* // WITH cte_name AS (SELECT id AS column1 FROM table_a) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* $qb = $conn->createQueryBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* ->with('cte_name', 'SELECT id AS column1 FROM table_a'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* // WITH cte_name(column1) AS (SELECT id AS column1 FROM table_a) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* $qb = $conn->createQueryBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* ->with('cte_name', 'SELECT id AS column1 FROM table_a', ['column1']); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* </code> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param string $name The name of the CTE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @param string[]|null $columns The optional columns list to select in the CTE. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
morozov marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* If not provided, the columns are inferred from the CTE. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @return $this This QueryBuilder instance. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @throws QueryException Setting an empty array as columns is not allowed. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public function with(string $name, string|QueryBuilder $part, ?array $columns = null): self | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe if the outer builder will ignore the parameters bound to the inner one, it shouldn't accept |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$this->commonTableExpressions[] = new CommonTableExpression($name, $part, $columns); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$this->sql = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return $this; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Specifies an item that is to be returned in the query result. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Replaces any previously specified selections, if any. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -1266,7 +1336,15 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw new QueryException('No SELECT expressions given. Please use select() or addSelect().'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return $this->connection->getDatabasePlatform() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$databasePlatform = $this->connection->getDatabasePlatform(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$selectParts = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (count($this->commonTableExpressions) > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$selectParts[] = $databasePlatform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
->createWithSQLBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
->buildSQL(...$this->commonTableExpressions); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$selectParts[] = $databasePlatform | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
->createSelectSQLBuilder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
->buildSQL( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
new SelectQuery( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -1281,6 +1359,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
$this->forUpdate, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return implode(' ', $selectParts); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Doctrine\DBAL\SQL\Builder; | ||
|
||
use Doctrine\DBAL\Query\CommonTableExpression; | ||
|
||
use function array_merge; | ||
use function count; | ||
use function implode; | ||
|
||
final class WithSQLBuilder | ||
{ | ||
public function buildSQL(CommonTableExpression $firstExpression, CommonTableExpression ...$otherExpressions): string | ||
{ | ||
$cteParts = []; | ||
|
||
foreach (array_merge([$firstExpression], $otherExpressions) as $part) { | ||
$ctePart = [$part->name]; | ||
if ($part->columns !== null && count($part->columns) > 0) { | ||
$ctePart[] = '(' . implode(', ', $part->columns) . ')'; | ||
} | ||
|
||
$ctePart[] = ' AS (' . $part->query . ')'; | ||
$cteParts[] = implode('', $ctePart); | ||
} | ||
|
||
return 'WITH ' . implode(', ', $cteParts); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code coverage complains about this check not beeing covered.
I'd say additional tests using
plain sql strings
as parts instead of QueryBuilder instances should be added anyway, and would cover this line here two (guessing).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a test with a sql string as cte query