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

Round MySQL Database Size to Nearest Integer to Prevent Precision Loss #182

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Round MySQL Database Size to Nearest Integer to Prevent Precision Loss #182

merged 1 commit into from
Jun 28, 2023

Conversation

mozex
Copy link
Contributor

@mozex mozex commented Jun 27, 2023

I've discovered an issue in the getMySQLDatabaseSize() method that can lead to PHP warnings about precision loss. Specifically, the MySQL query in this function employs the ROUND() function with one decimal place, sometimes resulting in a float. However, the function's type hint specifies an integer return type, and thus when the float is implicitly converted to an integer, PHP logs a warning about precision loss.

For example:

Implicit conversion from float-string "7.5" to int loses precision in /var/www/html/vendor/spatie/laravel-health/src/Support/DbConnectionInfo.php on line 60

This pull request proposes a simple but effective fix to this problem: adjusting the ROUND() function in the MySQL query to round to the nearest whole number, thereby ensuring that an integer is always returned.

The primary change in this PR includes:

Modification of the MySQL query within getMySQLDatabaseSize() to round to the nearest integer instead of returning a float.
This fix will ensure that the method maintains type consistency as per its integer type hint, preventing PHP from logging precision loss warnings. It will also uphold the integrity of the Laravel Health package, ensuring that it consistently delivers reliable and error-free performance.

I've conducted thorough testing of this solution and confirmed that it resolves the issue effectively, with the function continuing to deliver the expected output.

I would be grateful if you could review this change and consider it for inclusion in the next package release.

@freekmurze freekmurze merged commit 1268c60 into spatie:main Jun 28, 2023
@freekmurze
Copy link
Member

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