Fix a possible division by zero #634
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When you go to Symfony profiler -> Query Metrics and when Query time = 0ms there is an exception:
"An exception has been thrown during the rendering of a template ("Warning: Division by zero")."
[1/2] ContextErrorException: Warning: Division by zero
in vendor\doctrine\doctrine-bundle\DataCollector\DoctrineDataCollector.php at line 232
[2/2] Twig_Error_Runtime: An exception has been thrown during the rendering of a template ("Warning: Division by zero")
in vendor\doctrine\doctrine-bundle\Resources\views\Collector\db.html.twig at line 157
stof
reviewed
Mar 7, 2017
| foreach ($groupedQueries as $connection => $queries) { | ||
| foreach ($queries as $i => $query) { | ||
| $groupedQueries[$connection][$i]['executionPercent'] = $query['executionMS'] / $totalExecutionMS * 100; | ||
| $groupedQueries[$connection][$i]['executionPercent'] = | ||
| $this->executionTimePercentage($query['executionMS'], $totalExecutionMS ); |
There was a problem hiding this comment.
@stof the first parens is after 120 chars, so I think this was the right choice. @mikeSimonson you should remove the space after $totalExecutionMS.
e-moe
reviewed
Mar 27, 2017
| } | ||
| } | ||
|
|
||
| return $groupedQueries; | ||
| } | ||
|
|
||
| private function executionTimePercentage($executionTimeMS, $totalExecutionTimeMS) | ||
| { | ||
| if ($totalExecutionTimeMS === 0.0 || $totalExecutionTimeMS === 0) { |
There was a problem hiding this comment.
is it safe to compare float values like this? Maybe we can use very small epsilon value e.g. if ($totalExecutionTimeMS < $eps) where $eps = 1e-5 or something like this
There was a problem hiding this comment.
Yes, this comparison looks dangerous.
greg0ire
reviewed
Jul 15, 2017
| private function executionTimePercentage($executionTimeMS, $totalExecutionTimeMS) | ||
| { | ||
| if ($totalExecutionTimeMS === 0.0 || $totalExecutionTimeMS === 0) { | ||
| return 0; |
There was a problem hiding this comment.
Returning 100 would make more sense IMO
|
Is there any remaining issues which is blocking a merge? |
fabpot
pushed a commit
to fabpot/DoctrineBundle
that referenced
this pull request
Sep 29, 2017
* Fixed Division by zero when total execution time is 0ms
When you go to Symfony profiler -> Query Metrics and when Query time = 0ms there is an exception:
"An exception has been thrown during the rendering of a template ("Warning: Division by zero")."
[1/2] ContextErrorException: Warning: Division by zero
in vendor\doctrine\doctrine-bundle\DataCollector\DoctrineDataCollector.php at line 232
[2/2] Twig_Error_Runtime: An exception has been thrown during the rendering of a template ("Warning: Division by zero")
in vendor\doctrine\doctrine-bundle\Resources\views\Collector\db.html.twig at line 157
* Fix division by zero in the data collector
fabpot
added a commit
that referenced
this pull request
Oct 4, 2017
* 1.6.x: Fix deprecation with Symfony 3.4 fixed tests fixed compat with PHP 5.5 removed unused use statement changed services to be public for compat with Symfony 4 Fix check for DoctrineType::reset() existence Fixed for SF > 3.0 on shutdown container create and close connections Add "kernel.reset" tag to "form.type.entity" Bugfixes to CreateDatabaseCommand and its tests, fixes #686 fixes doctrine/orm#6689 Removing extra space Fix a possible division by zero (#634) Add autowiring for Doctrine\DBAL\Connection (#685) Use ChildDefinition class when available
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.