laravel-debugbar icon indicating copy to clipboard operation
laravel-debugbar copied to clipboard

Support ability to sort queries by alphabetical order

Open AlexJump24 opened this issue 1 year ago • 11 comments

Hello,

In a previous PR meant to thank you for the brilliant package that has been so useful over the years!

I'm putting in this PR in the hope it will be useful to help resolve a problem I've been facing when looking through the queries tab on the debugbar to debug duplicate N+1 queries. On the particular project I was debugging there were a lot of duplicates and I was finding it difficult to determine which queries were appearing the most as duplicates and how many there were of them.

So I had an idea to maybe support the ability of sorting the queries collector by a custom sort, in this case alphabetical order. Testing this help really helped me quickly identify which queries were having the most N+1 issues.

A few thought processes I had.

  1. I wasn't a huge fan of having a string key in the config, when everything is pretty much driven by booleans, however I thought there would be potential to define multiple custom sort options going forward e.g. sort by query execution time if you want to look through queries in order that way.
  2. When initially testing I was simply ordering the query but of course this just ordered directly via the selected columns, I feel the most suitable option here would be to sort via everything after the table name. Here I have grabbed everything after the word FROM which from my understanding should always be present as the Laravel query builder requires a table? Initial testing on a few projects this seemed to work nicely and grouped duplicate queries alongside one another in their execution order.
  3. Naming was hard, and happy to change things if required.
  4. I have left the default sort to be execution_order which completely bypasses the usort so there is no performance degradation by default.
image

Hopefully this is something that is suitable to add into the package as I think it would add a lot of value debugging and as previously mentioned open scope to sorting in order ways.

Cheers.

AlexJump24 avatar Sep 11 '24 19:09 AlexJump24

Wouldn't this affect the waterfall timeline?

parallels999 avatar Sep 11 '24 19:09 parallels999

Wouldn't this affect the waterfall timeline?

I've given this a quick test and the timeline appears to be the same, I may be missing something though 👍🏼

AlexJump24 avatar Sep 11 '24 19:09 AlexJump24

I mean, each query have startTime/endTime, the graph is generated based on that, if the order is changed, there would no longer be a waterfall.

https://github.com/barryvdh/laravel-debugbar/blob/bae4b221e725c52deffceb9fe537928b210c9921/src/DataCollector/QueryCollector.php#L159-L162

parallels999 avatar Sep 11 '24 19:09 parallels999

https://github.com/maximebf/php-debugbar/pull/410 Also could be sorted by

  • Memory use
  • Duration

parallels999 avatar Sep 11 '24 20:09 parallels999

I mean, each query have startTime/endTime, the graph is generated based on that, if the order is changed, there would no longer be a waterfall.

https://github.com/barryvdh/laravel-debugbar/blob/bae4b221e725c52deffceb9fe537928b210c9921/src/DataCollector/QueryCollector.php#L159-L162

I think this can't happen as the sort doesn't affect the start time/end time and in the TimeDataCollector there the waterfall order is driven via the usort that sorts via the start time https://github.com/maximebf/php-debugbar/blob/master/src/DebugBar/DataCollector/TimeDataCollector.php#L228

AlexJump24 avatar Sep 12 '24 05:09 AlexJump24

the waterfall order is driven via the usort that sorts via the start time https://github.com/maximebf/php-debugbar/blob/master/src/DebugBar/DataCollector/TimeDataCollector.php#L228

https://github.com/barryvdh/laravel-debugbar/blob/bae4b221e725c52deffceb9fe537928b210c9921/src/DataCollector/QueryCollector.php#L480 QueryCollector overwrite collect(), I have no idea why you bring it to the table.

I didn't mean that, but never mind, I'm not the one who does the merges anyway.

It seems that you have not used the timeline option in QueryCollector, set 'duration_background' => true,and se the magic, also 'timeline' => true, https://github.com/barryvdh/laravel-debugbar/blob/bae4b221e725c52deffceb9fe537928b210c9921/config/debugbar.php#L223 https://github.com/barryvdh/laravel-debugbar/blob/bae4b221e725c52deffceb9fe537928b210c9921/config/debugbar.php#L222

parallels999 avatar Sep 12 '24 14:09 parallels999

Not 100% sure what you are trying to say still but now I realise it makes no sense to keep doing it on the addQuery rather than the collect. I have been testing with the two configuration options set to true and pre and post change the timeline is rendering exactly the same.

Thanks.

AlexJump24 avatar Sep 12 '24 14:09 AlexJump24

When you say "timeline", you are looking at Timelime Tab? or at The Queries Tab? because all the time I have been talking about the queries tab, there is a timeline waterfall on queries tab

parallels999 avatar Sep 12 '24 14:09 parallels999

When you say "timeline", you are looking at Timelime Tab? or at The Queries Tab? because all the time I have been talking about the queries tab, there is a timeline waterfall on queries tab

I see what you mean now thanks I had never even realised that was a thing on there! Yeah you are correct it is affected on there, although I guess that would be expected in this context if you changed the sort. Would the waterfall be needed if sorted, as you said I was looking at the timeline tab where it is still visible. If not resolvable no problem I will just tweak things when I require but thought it would be a useful potential feature even if my implementation doesn't end up being suitable.

AlexJump24 avatar Sep 12 '24 14:09 AlexJump24

Also there is database events, like transaction,commit,rollback and others, these separate query blocks based on their execution time. What would happen to them in this order?

https://github.com/barryvdh/laravel-debugbar/blob/bae4b221e725c52deffceb9fe537928b210c9921/src/DataCollector/QueryCollector.php#L441-L454 https://github.com/barryvdh/laravel-debugbar/blob/bae4b221e725c52deffceb9fe537928b210c9921/src/LaravelDebugbar.php#L372-L424

parallels999 avatar Sep 12 '24 15:09 parallels999

I was finding it difficult to determine which queries were appearing the most as duplicates

There is a button to show only duplicates image

angeljqv avatar Oct 31 '24 20:10 angeljqv