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

Add QueryBuilder support for UNION clause #6369

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

sbuerk
Copy link
Contributor

@sbuerk sbuerk commented Apr 25, 2024

Q A
Type feature
Fixed issues #6368

Summary

The UNION operator is used to combine the result-set
of two or more SELECT statements, which all database
vendors supports with usual specialities for each.

Still, there is a common shared subset which works for
all of them:

    SELECT column_name(s) FROM table1
    WHERE ...

    UNION <ALL | DISTINCT>

    SELECT column_name(s) FROM table2
    WHERE ...

    ORDER BY ...
    LIMIT x OFFSET y

with shared common requirements:

  • Each SELECT must return the same fields
    in number, naming and order.

  • Each SELECT must not have ORDER BY,
    expect MySQL allowing it to be used as sub
    query expression encapsulated in parenthesis.

It is now possible to build UNION queries using
following additional QueryBuilder API methods:

  • union(string|QueryBuilder $part) to create a UNION
    query retrieving unique rows
  • addUnion(string|QueryBuilder $part, UnionType $type)
    to add UNION (ALL|DISTINCT) query with the selected
    union query type.

This follows the generic logic of select(...) and
addSelect(...) along with introducing new UnionType
enum and internal QueryType::UNION enum case.

Technically, the SQL build process is dispatched to a
DefaultUnionSQLBuilder along with an UnionSQLBuilder
interface, which also allows application to implement
custom behaviour if required. Union SQL keyword and part
SQL generation is handled through added methods on the
Platforms to allow adjustment for furture versions if
needed - or throw a Exception if a Platform does not
support it anymore.

Example:

$platform = $connection->getDatabasePlatform();
$qb       = $>connection->createQueryBuilder();
$select10 = $platform->getDummySelectSQL('2 as field_one');
$select20 = $platform->getDummySelectSQL('1 as field_one');
$qb->union($select10)
   ->addUnion($select20, UnionType::ALL)
   ->setMaxResults(1)
   ->setFirstResult(1)
   ->orderBy('field_one', 'ASC');
$rows = $qb->executeQuery()->fetchAllAssociative();

Unit and functional tests are added to demonstrate the
implementation and cover it for future changes.

Resolves: #6368

@sbuerk sbuerk force-pushed the union-support branch 2 times, most recently from d1f6bc8 to 4013072 Compare April 25, 2024 09:34
docs/en/reference/query-builder.rst Outdated Show resolved Hide resolved
src/SQL/Builder/DefaultUnionSQLBuilder.php Outdated Show resolved Hide resolved
@sbuerk sbuerk force-pushed the union-support branch 5 times, most recently from c5f2b12 to 71c4d7a Compare April 25, 2024 15:23
@sbuerk sbuerk requested a review from derrabus April 26, 2024 09:30
@stof
Copy link
Member

stof commented Jun 10, 2024

The current API with ->addUnion() and ->addUnionAll() is confusing IMO. The implementation will remember the DISTINCT/ALL behavior of the last call and apply it to all unions being added. If the SQL standard does not allow mixing both modes when having several union clauses, a separate flag would be a much better API.

I'm also wondering whether the QueryBuilder should throw exceptions when using incompatible methods together (adding select and where clauses and then using addUnion would silently ignore the select and where clauses right now, while the proper usage is to put those on the query builder used inside the union clause)

@derrabus derrabus added this to the 4.1.0 milestone Jun 12, 2024
@sbuerk sbuerk force-pushed the union-support branch 11 times, most recently from 5aab898 to 611d3d8 Compare June 13, 2024 10:05
@sbuerk sbuerk changed the title Add rudimentary UNION support to the QueryBuilder Extend QueryBuilder to support UNION clause Jun 13, 2024
@sbuerk sbuerk changed the title Extend QueryBuilder to support UNION clause Add QueryBuilder support for UNION clause Jun 13, 2024
@sbuerk
Copy link
Contributor Author

sbuerk commented Jun 13, 2024

The current API with ->addUnion() and ->addUnionAll() is confusing IMO. The implementation will remember the DISTINCT/ALL behavior of the last call and apply it to all unions being added. If the SQL standard does not allow mixing both modes when having several union clauses, a separate flag would be a much better API.

I'm also wondering whether the QueryBuilder should throw exceptions when using incompatible methods together (adding select and where clauses and then using addUnion would silently ignore the select and where clauses right now, while the proper usage is to put those on the query builder used inside the union clause)

@stof Thanks for your feedback.

The standard in fact allows to mix UNION DISTINCT and UNION ALL to add the next part, which was not reflected in the first API implementation.

I changed and further reduced that to union() and addUnion() methods, now dealing only with one part. The addUnion() enforces to define the time for the added part using a enum class. Not sure if a boolean would be more favourized over the enum, while I'm personally think the enum is more speaking in that case).

After talking with @derrabus about this PR, I moved the SQL generation into the platform classes and surround union parts on platforms supporting it (default), removing it for SQLite for now.

Platforms behave differently for additional stuff on the union select parts, which are mostly reduced using the part parenthesis surrounding excpet sqlite. Due to the nature that either string or QueryBuilder parts could be added, it is hard to scan for platform incompatible part queries - and left to be reported from the database. We have at least an excpetion in place if only one part is provided.

Not sure if we should really throw an exception if unsupported properties are set for the outer union query (for example set() or values() which is not done yet. I'd say this should be done in a dedicated change to throw exception for all types if unsupported properties and information are set beforehand and not properly cleared. What do you think ?

Not sure about the codecoverage upload error in the checks or what is needed to do to make it happy.

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

LGTM, so I start nit-picking. 😉

tests/Query/QueryBuilderTest.php Outdated Show resolved Hide resolved
The `UNION` operator is used to combine the result-set
of two or more `SELECT` statements, which all database
vendors supports with usual specialities for each.

Still, there is a common shared subset which works for
all of them:

```
    SELECT column_name(s) FROM table1
    WHERE ...

    UNION <ALL | DISTINCT>

    SELECT column_name(s) FROM table2
    WHERE ...

    ORDER BY ...
    LIMIT x OFFSET y
```

with shared common requirements:

* Each `SELECT` must return the same fields
  in number, naming and order.

* Each `SELECT` **must not** have `ORDER BY`,
  expect MySQL allowing it to be used as sub
  query expression encapsulated in parenthesis.

It is now possible to build `UNION` queries using
following additional QueryBuilder API methods:

* `union(string|QueryBuilder $part)` to create a `UNION`
   query retrieving unique rows
* `addUnion(string|QueryBuilder $part, UnionType $type)`
   to add `UNION (ALL|DISTINCT)` query with the selected
   union query type.

This follows the generic logic of `select(...)` and
`addSelect(...)` along with introducing new UnionType
enum and internal QueryType::UNION enum case.

Technically, the SQL build process is dispatched to a
`DefaultUnionSQLBuilder` along with an `UnionSQLBuilder`
interface, which also allows application to implement
custom behaviour if required. Union SQL keyword and part
SQL generation is handled through added methods on the
Platforms to allow adjustment for furture versions if
needed - or throw a Exception if a Platform does not
support it anymore.

Example:

```php
$platform = $connection->getDatabasePlatform();
$qb       = $>connection->createQueryBuilder();
$select10 = $platform->getDummySelectSQL('2 as field_one');
$select20 = $platform->getDummySelectSQL('1 as field_one');
$qb->union($select10)
   ->addUnion($select20, UnionType::ALL)
   ->setMaxResults(1)
   ->setFirstResult(1)
   ->orderBy('field_one', 'ASC');
$rows = $qb->executeQuery()->fetchAllAssociative();
```

Unit and functional tests are added to demonstrate the
implementation and cover it for future changes.

Resolves: doctrine#6368
@derrabus derrabus merged commit ff0dab7 into doctrine:4.1.x Jun 14, 2024
76 of 77 checks passed
derrabus added a commit that referenced this pull request Jun 18, 2024
|      Q       |   A
|------------- | -----------
| Type         | improvement
| Fixed issues | Follows #6369

#### Summary

I think that people usually want to have a distinct union, if they build
a union query. This would also be in line with SQL where a `UNION`
without any additional keywords would produce a distinct union.

cc @sbuerk
derrabus added a commit to derrabus/dbal that referenced this pull request Jun 19, 2024
* 4.1.x: (25 commits)
  Simplify signature of fetchTableOptionsByTable
  Add MariaDb1010Platform for fetchTableOptionsByTable
  PHPUnit 10.5.21 (doctrine#6447)
  Move schema split for SQLite CREATE INDEX only (doctrine#6352)
  PHPStan 1.11.5 (doctrine#6446)
  Default to distinct union queries (doctrine#6439)
  Revert "Merge pull request doctrine#6413 from achterin/bugfix/foreign_key_name_change_detection"
  Add `QueryBuilder` support for `UNION` clause (doctrine#6369)
  CI: Update MariaDB versions (doctrine#6426)
  CI MariaDB: add 11.4, remove 11.0 (doctrine#6432)
  Display warnings when running PHPUnit in CI (doctrine#6431)
  Fix typo in the portability documentation (doctrine#6430)
  Fix MariaDB fetching of default table character-set (doctrine#6361) (doctrine#6425)
  Fix the portability documentation (doctrine#6429)
  Update tests/Platforms/AbstractPlatformTestCase.php
  Update tests/Platforms/AbstractPlatformTestCase.php
  add test
  Fix: Skip type comparison if disableTypeComments is true
  Remove redundant variable (doctrine#6326)
  Fix test names to reflect their actual purpose
  ...
@sbuerk sbuerk deleted the union-support branch August 13, 2024 16:00
reviewtypo3org pushed a commit to TYPO3/typo3 that referenced this pull request Aug 16, 2024
The `UNION Clause` is used to combine the result-set of
two or more `SELECT` SQL queries suported by all database
vendors - at least on a shared basic level with the usual
special vendor enhancements.

There is a common shared subset working for all of them:

    SELECT column_name(s) FROM table1
    WHERE ...

    UNION <ALL | DISTINCT>

    SELECT column_name(s) FROM table2
    WHERE ...

    ORDER BY ...
    LIMIT x OFFSET y

Instead of introducing the `UNION Clause` support into
the TYPO3 QueryBuilder, the challenge was taken to add
this directly into Doctrine DBAL [1]. The effort was
rewarded and with the release of `Doctrine DBAL 4.1.0`
`UNION Clause` [2] the support is included.

This change adopts the new feature into the extended
`ConcreteQueryBuilder` and `QueryBuilder` to support
extension authors with a simple and usable interface
to build `UNION (DISTINCT)` and `UNION ALL` queries.

[1] doctrine/dbal#6369
[2] https://github.com/doctrine/dbal/releases/tag/4.1.0

Resolves: #104631
Related: #104628
Releases: main
Change-Id: I443b762fdc6a9f1ed77b3d655d0ab2f371a56d50
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83943
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
TYPO3IncTeam pushed a commit to TYPO3-CMS/core that referenced this pull request Aug 16, 2024
The `UNION Clause` is used to combine the result-set of
two or more `SELECT` SQL queries suported by all database
vendors - at least on a shared basic level with the usual
special vendor enhancements.

There is a common shared subset working for all of them:

    SELECT column_name(s) FROM table1
    WHERE ...

    UNION <ALL | DISTINCT>

    SELECT column_name(s) FROM table2
    WHERE ...

    ORDER BY ...
    LIMIT x OFFSET y

Instead of introducing the `UNION Clause` support into
the TYPO3 QueryBuilder, the challenge was taken to add
this directly into Doctrine DBAL [1]. The effort was
rewarded and with the release of `Doctrine DBAL 4.1.0`
`UNION Clause` [2] the support is included.

This change adopts the new feature into the extended
`ConcreteQueryBuilder` and `QueryBuilder` to support
extension authors with a simple and usable interface
to build `UNION (DISTINCT)` and `UNION ALL` queries.

[1] doctrine/dbal#6369
[2] https://github.com/doctrine/dbal/releases/tag/4.1.0

Resolves: #104631
Related: #104628
Releases: main
Change-Id: I443b762fdc6a9f1ed77b3d655d0ab2f371a56d50
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83943
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants