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

feat: Optimize countQuery with complex select #3008

Merged
merged 18 commits into from
Jun 27, 2023

Conversation

Arkhas
Copy link
Contributor

@Arkhas Arkhas commented Jun 5, 2023

When performing subqueries, some of the keywords used to determine if a query is complexe can be found in subqueries inside the main query :

protected function isComplexQuery($query): bool
{
     return Str::contains(Str::lower($query->toSql()), ['union', 'having', 'distinct', 'order by', 'group by']);
}

For exemple :

select 
    "users".*,
    (select id from posts where posts.user_id = users.id order by created_at desc limit 1) as last_post_id
from "users"

This query was detected as complexe, but the subquery had no impact on the number of records returns,
The subquery was executed and had a huge impact on performance

To force this behavior, you can use the ignoreSelectsInCountQuery() on a QueryDatatable :

 $dataTable = app('datatables')->of(
            DB::table('users')
                ->select('users.*')
                ->addSelect([
                    'last_post_id' => DB::table('posts')
                                        ->whereColumn('posts.user_id', 'users.id')
                                        ->orderBy('created_at')
                                        ->select('id'),
                ])
                ->orderBy(DB::table('posts')->whereColumn('posts.user_id', 'users.id')->orderBy('created_at')->select('created_at')
                )
        )->ignoreSelectsInCountQuery();

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@yajra yajra requested a review from hpacleb June 7, 2023 04:49
src/QueryDataTable.php Outdated Show resolved Hide resolved
@Arkhas
Copy link
Contributor Author

Arkhas commented Jun 12, 2023

ping @yajra

@yajra yajra changed the title Optimize countQuery with complexe select feat: Optimize countQuery with complex select Jun 14, 2023
src/QueryDataTable.php Outdated Show resolved Hide resolved
src/QueryDataTable.php Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Arkhas
Copy link
Contributor Author

Arkhas commented Jun 20, 2023

I am struggling every time with the CI, does StyleCI have an automated fixer ? It would be much easier :)

@yajra
Copy link
Owner

yajra commented Jun 20, 2023

I am struggling every time with the CI, does StyleCI have an automated fixer ? It would be much easier :)

No problem with StyleCI, I can automate the PR afterwards.

@Arkhas Arkhas requested a review from yajra June 20, 2023 12:44
@yajra
Copy link
Owner

yajra commented Jun 22, 2023

There is still a Static Analysis issue:

 ------ --------------------------------------------- 
  Line   QueryDataTable.php                           
 ------ --------------------------------------------- 
  168    Negated boolean expression is always false.  
 ------ --------------------------------------------- 

But let me check again further. If it's a false positive, maybe we can just add it to ignored errors?

@Arkhas
Copy link
Contributor Author

Arkhas commented Jun 22, 2023

yeah I don't understand why it is detected like that

@Arkhas
Copy link
Contributor Author

Arkhas commented Jun 26, 2023

where you able to check it ? Or do you need more explanation on what the function does ?

@yajra yajra merged commit 65d89f2 into yajra:master Jun 27, 2023
@yajra
Copy link
Owner

yajra commented Jun 27, 2023

Released on v10.4.4 🚀 including the fixes for phpstan. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants