MDEV-37939 Add function context to division by zero warnings#4569
MDEV-37939 Add function context to division by zero warnings#4569alexaustin007 wants to merge 7 commits intoMariaDB:mainfrom
Conversation
|
|
gkodinov
added
the
External Contribution
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
This is a preliminary review. The approach is sound IMHO, but there's some things missing (see the comments below).
Also, please make sure you re-record the rest of the failing test cases too.
|
Thank you for the suggestion @gkodinov . I removed the no arg signal_divide_by_null() and made the remaining method protected |
|
Updated it so LOG/LN/LOG2/LOG10 now emit the correct warning (Invalid argument for logarithm) instead of Division by 0. Division/DIV/MOD remain unchanged also reordered the expanded test of func_math_div_zero to verify empty warnings and all cases. Build and test pass. @gkodinov @vuvova Requesting to review. Thank you |
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Something interesting from https://dev.mysql.com/doc/refman/8.4/en/mathematical-functions.html#function_log2:
If X is less than or equal to 0.0E0, the function returns NULL and a warning “Invalid argument for logarithm” is reported. Returns NULL if X is NULL.
I think this is a good idea. But I will leave this to the final reviewer.
|
@gkodinov Updated the log warnings to use specific messages instead of "division by 0", removed the dead code, and refreshed the tests/results. Requesting to review |
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
LGTM. Please wait for the final review.
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
There are failing tests because of the new warnings. Please fix.
Re-recorded test results for the updated LOG domain warnings. All related tests now pass. The main.tmp_space_usage failure in the MSAN build seems unrelated issue |
|
Thanks, there's also some conflicts in the error message file. Please resolve. |
Add contextual warnings for LOG/LN/LOG2/LOG10 while preserving error code 1365 for compatibility. Make divide by zero reporting explicit at call sites and expand tests to cover div/mod with real, decimal, and string arguments.
Replace division by zero warnings for LOG/LN/LOG2/LOG10 with ER_INVALID_ARGUMENT_FOR_LOGARITHM while keeping division warnings unchanged. Reorder and expand the test to verify empty warnings, log warnings, and division warnings.
Replace “division by 0” warnings for LOG/LN/LOG2/LOG10 with specific logarithm domain warnings and keep division/mod paths on ER_DIVISION_BY_ZERO. Update func_math_div_zero test and expected results accordingly.
alexaustin007
force-pushed
the
MDEV-37939
branch
from
af74b67 to
79d6395
Compare
3 participants
JIRA: https://jira.mariadb.org/browse/MDEV-37939
Summary
Testing