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

Feature. QueryBuilder. Query union. #6015

Merged
merged 3 commits into from
May 28, 2022

Conversation

iRedds
Copy link
Collaborator

@iRedds iRedds commented May 22, 2022

Description
This is the second attempt to add a query union to QueryBuilder.
Ref #4291

The implementation provides two methods:

  • BaseBuilder::union(BaseBuilder|Closure $union)
  • BaseBuilder::unionAll(BaseBuilder|Closure $union)

The union() method can be called in any order relative to the main query, but the union query will always be appended to the end. That is, the LIMIT or ORDER BY clauses will be relative to the main query.

$builder->union($union)->limit(1)->orderBy('id');
// SELECT * FROM table LIMIT 1 ORDER BY id UNION query

Since DBMSs (MSSQL and Oracle) are demanding on queries using LIMIT and ORDER BY, when generating SQL, all queries are wrapped in a SELECT * FROM(....) alias query.
The alias uwrp0 is used for the main query. Each subsequent query will have an alias with index +1.

$builder->union($union)->limit(1)->orderBy('id')->unionAll($union2);
// SELECT * (SELECT * FROM table LIMIT 1 ORDER BY id) uwrp0 
// UNION SELECT * FROM (query) uwrp1 
// UNION ALL SELECT * FROM (query2) uwrp2

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@iRedds
Copy link
Collaborator Author

iRedds commented May 23, 2022

I don't think replacing assertEquals() with assertSame() is the correct behavior for checking code style.

@kenjis
Copy link
Member

kenjis commented May 23, 2022

With this PR, I understand we can't write a query to use an ORDER BY or LIMIT clause to sort or limit the entire UNION result like the following. Am I correct?

(SELECT a FROM t1 WHERE a=10 AND B=1)
UNION
(SELECT a FROM t2 WHERE a=11 AND B=2)
ORDER BY a LIMIT 10;

@iRedds
Copy link
Collaborator Author

iRedds commented May 23, 2022

Yes, such code is not possible by default. But it can be wrapped in another query using newQuery()->fromSubquery().
Like in a live test.

$union   = $this->db->table('user')->limit(1)->orderBy('id', 'ASC');

$builder = $this->db->table('user')->union($union)->limit(1)->orderBy('id', 'DESC');

$result   = $this->db->newQuery()->fromSubquery($builder, 'q')->orderBy('id', 'DESC')->get();
SELECT * FROM (
    SELECT * FROM (SELECT * FROM user ORDER BY id DESC LIMIT 1)
    UNION
    SELECT * FROM (SELECT * FROM user ORDER BY id ASC LIMIT 1)
) q ORDER BY id

@iRedds iRedds force-pushed the feature/query-builder-union branch from 3610fc9 to c257634 Compare May 24, 2022 13:27
@kenjis kenjis added the enhancement PRs that improve existing functionalities label May 25, 2022
@kenjis
Copy link
Member

kenjis commented May 25, 2022

I would like the test code to follow the Arrange Act Assert pattern as much as possible.
http://wiki.c2.com/?ArrangeActAssert

There are many test codes that do not follow it, though.

@@ -72,6 +72,7 @@ Database
- QueryBuilder raw SQL string support
- Added the class ``CodeIgniter\Database\RawSql`` which expresses raw SQL strings.
- :ref:`select() <query-builder-select-rawsql>`, :ref:`where() <query-builder-where-rawsql>`, :ref:`like() <query-builder-like-rawsql>`, :ref:`join() <query-builder-join-rawsql>` accept the ``CodeIgniter\Database\RawSql`` instance.
- QueryBuilder. Union queries.
Copy link
Member

Choose a reason for hiding this comment

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

If there is a link to the detailed page, readers can go and see it easily.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

@kenjis
Copy link
Member

kenjis commented May 25, 2022

@iRedds I think the query example to use an ORDER BY or LIMIT clause to sort or limit the entire UNION result should be documented.
And there are conflicts in the documentation.
Can you rebase this PR?

Signed-off-by: Andrey Pyzhikov <5071@mail.ru>
@iRedds iRedds force-pushed the feature/query-builder-union branch from c257634 to a3ccae4 Compare May 25, 2022 07:48
@iRedds
Copy link
Collaborator Author

iRedds commented May 25, 2022

@kenjis I've added an example to the documentation, but now @lonnieezell has nothing to add to the new version of the book )))

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

Thank you!

@kenjis
Copy link
Member

kenjis commented May 25, 2022

@iRedds Do you say about https://leanpub.com/codeigniter4foundations ?

@iRedds
Copy link
Collaborator Author

iRedds commented May 25, 2022

@kenjis yes, i do

@kenjis kenjis added the database Issues or pull requests that affect the database layer label May 27, 2022
@kenjis kenjis requested a review from michalsn May 27, 2022 06:40
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

Excellent addition.

There are small additions to the docs I would like to see, though.

user_guide_src/source/database/query_builder.rst Outdated Show resolved Hide resolved
user_guide_src/source/database/query_builder.rst Outdated Show resolved Hide resolved
iRedds and others added 2 commits May 27, 2022 16:57
Clarification for union() method.

Co-authored-by: Michal Sniatala <michal@sniatala.pl>
Clarification for unionAll() method.

Co-authored-by: Michal Sniatala <michal@sniatala.pl>
@kenjis kenjis merged commit c6ace5d into codeigniter4:develop May 28, 2022
@iRedds
Copy link
Collaborator Author

iRedds commented May 28, 2022

Thanks to everyone who helped along this long journey.

@iRedds iRedds deleted the feature/query-builder-union branch May 28, 2022 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Issues or pull requests that affect the database layer enhancement PRs that improve existing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants