MDEV-37939 Add function context to division by zero warnings #4569

alexaustin007 · 2026-01-21T02:36:54Z

JIRA: https://jira.mariadb.org/browse/MDEV-37939

Summary

Add contextual “Division by 0 in function” warnings for LOG/LN/LOG2/LOG10
Keep warning code ER_DIVISION_BY_ZERO (1365) unchanged
Add a regression test for LOG-family warnings vs generic division warnings

Testing

MTR_BINDIR=../build ./mariadb-test-run.pl --suite=main --record func_math_div_zero
MTR_BINDIR=../build ./mariadb-test-run.pl --suite=main func_math_div_zero

CLAassistant · 2026-01-21T02:37:01Z

All committers have signed the CLA.

gkodinov

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.

sql/item_func.cc

gkodinov · 2026-01-21T08:46:15Z

sql/item_func.h

  }
  String *val_str_from_val_str_ascii(String *str, String *str2);

  void signal_divide_by_null();


ditto here: remove the old one.
Also, please consider moving this under "protected": let's make sure it's not used anywhere externally.
I'd even go as far as making it an inline static function in item_func.cc: it doesn't have to be a class method. I'd at least make it static to highlight that it's just a utility function.

mysql-test/main/func_math_div_zero.result

gkodinov · 2026-01-21T08:57:40Z

sql/share/errmsg-utf8.txt

 ER_WARN_NO_IMPLICIT_QB_NAMES_IN_VIEW
 	eng "Implicit query block names are ignored for hints specified within VIEWs"
+ER_DIVISION_BY_ZERO_IN_FUNC 22012
+        eng "Division by 0 in function %s"


Since you are at it, I'd even add a specific cause for the error. Some functions could throw this error on several occasions.

Just to let you know, I haven’t touched this in the current commit

To get the final review approval, I think you also should address Sergei's question.

The current changes should address Sergei's question now.

mysql-test/main/func_math_div_zero.result

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.

alexaustin007 · 2026-01-22T05:22:59Z

Thank you for the suggestion @gkodinov . I removed the no arg signal_divide_by_null() and made the remaining method protected
All call sites now pass explicit context and also expanded func_math_div_zero` to cover DIV/MOD with decimal/string args and re‑recorded the result file.
I still haven't added the specific cause for the error, once this looks good, I can cover up in my next steps.

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.

alexaustin007 · 2026-01-22T12:15:49Z

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

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 · 2026-01-22T12:30:38Z

sql/item_func.cc


+namespace {
+
+void push_divide_by_zero_warning(THD *thd)


It's a but of an overkill these helper functions. push_warning itself is not considered too much IMHO. It's OK to leave it like this. Just saying I wouldn't have went to the trouble. I was hoping that you can either abandon Item_func::signal_... altogether or use local static helper functions. Now you're doing both. This is definitely redundant.

gkodinov · 2026-01-22T12:36:54Z

sql/item_func.cc

+{
+  push_warning(thd, Sql_condition::WARN_LEVEL_WARN,
+               ER_INVALID_ARGUMENT_FOR_LOGARITHM,
+               ER_THD(thd, ER_INVALID_ARGUMENT_FOR_LOGARITHM));


Heh, you've found yourself a dead error code! This code is not used anywhere currently.
I'd personally prefer to leave it like that and add a new one that actually helps, e.g. "passing a negative argument to ln produces undefined results".

Otherwise we're back to the situation before your change: you get a cryptic error message that you can't really pinpoint to any specific part of a complex expression.

I will try to offer what I think is a meaningful error message in each case below.

gkodinov · 2026-01-22T12:40:32Z

sql/item_func.cc

  if (value <= 0.0)
  {
-    signal_divide_by_null();
+    signal_invalid_log_argument();


Message: Passing negative argument to ln() makes the function's result undefined.

gkodinov · 2026-01-22T12:40:45Z

sql/item_func.cc

  if (value <= 0.0)
  {
-    signal_divide_by_null();
+    signal_invalid_log_argument();


Message: Passing negative argument to log() makes the function's result undefined.

gkodinov · 2026-01-22T12:43:10Z

sql/item_func.cc

    if (value2 <= 0.0 || value == 1.0)
    {
-      signal_divide_by_null();
+      signal_invalid_log_argument();


Message: Passing negative 2nd argument to log() makes the function's result undefined.

gkodinov · 2026-01-22T12:44:01Z

sql/item_func.cc

  if (value <= 0.0)
  {
-    signal_divide_by_null();
+    signal_invalid_log_argument();


Message: Passing negative argument to log2() makes the function's result undefined.

gkodinov · 2026-01-22T12:44:13Z

sql/item_func.cc

  if (value <= 0.0)
  {
-    signal_divide_by_null();
+    signal_invalid_log_argument();


Message: Passing negative argument to log10() makes the function's result undefined.

MDEV-37939 Add function context to division by zero warnings

f01e68f

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Jan 21, 2026

gkodinov requested changes Jan 21, 2026

View reviewed changes

gkodinov requested changes Jan 22, 2026

View reviewed changes

Uh oh!

MDEV-37939 Add function context to division by zero warnings #4569

Are you sure you want to change the base?

MDEV-37939 Add function context to division by zero warnings #4569

Conversation

alexaustin007 commented Jan 21, 2026

Uh oh!

CLAassistant commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alexaustin007 commented Jan 22, 2026

Uh oh!

alexaustin007 commented Jan 22, 2026

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

CLAassistant commented Jan 21, 2026 •

edited

Loading