Skip to content
/ server Public

MDEV-38670 Unary minus on empty string returns -0 #4632

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.

Sign up for GitHub

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

Open
3titoo wants to merge 1 commit into
base: main
Choose a base branch
from
+24 −1
Open

MDEV-38670 Unary minus on empty string returns -0 #4632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions mysql-test/main/func_math.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3739,3 +3739,12 @@ cast(1 as unsigned) - cast(2 as unsigned)
-1
set sql_mode=default;
# End of 10.5 tests
# MDEV-38670 Unary minus on empty string should return 0
Copy link
Member

@gkodinov gkodinov Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func_math is failing on most of the build-bot platforms. Please fix this.

@x := -''
0
# Ensure binary subtraction matches
@y := 0 - ''
0
# Additional edge cases
-0.0 -'0' -' ' -@x --@x
0.0 0 0 0 0
7 changes: 7 additions & 0 deletions mysql-test/main/func_math.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1998,3 +1998,10 @@ select cast(1 as unsigned) - cast(2 as unsigned);
set sql_mode=default;

--echo # End of 10.5 tests
Copy link
Member

@gkodinov gkodinov Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is customary to end a test file with a version test end echo command. Please add one for the target version: 13 in your case I believe, unless the final reviewer requests otherwise.


--echo #
--echo # MDEV-38670 Unary minus on empty string should return 0
--echo #
SELECT @x := -'';
SELECT @y := 0 - '';
SELECT -0.0, -'0', -' ', -@x, --@x;
Copy link
Member

@gkodinov gkodinov Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, these are not floating point literals. The tests with these are irrelevant.

On how to specify constants in MariaDB, see:

MariaDB [test]> create table t1 as select 0 as a, 0.0 as b, 0e0 as c;
Query OK, 1 row affected (0.042 sec)
Records: 1  Duplicates: 0  Warnings: 0

MariaDB [test]> show create table t1;
+-------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                                                        |
+-------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| t1    | CREATE TABLE `t1` (
  `a` int(1) NOT NULL,
  `b` decimal(2,1) NOT NULL,
  `c` double NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci |
+-------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.001 sec)

Please add relevant tests that cover all of your changes and nothing but.

9 changes: 8 additions & 1 deletion sql/item_func.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1825,7 +1825,11 @@ double Item_func_neg::real_op()
{
double value= args[0]->val_real();
null_value= args[0]->null_value;
return -value;
double res= -value;
Copy link
Member

@gkodinov gkodinov Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very good start. But it's definitely not all there is to it.
This will only cover the negation function which you happen to be using.

The problem is more wide-spread. It's everywhere a FP literal is used.
E.g. try "select 0e0 * -1e0".

I believe this is happening because MySQL does IEEE 754 arithmetic. And doesn't account for "the signed zero" (https://en.wikipedia.org/wiki/Signed_zero) phenomenon while doing it.

As a result many operations done on the real and double data types internally can result in a signed zero.

The question now is where do you want to stop with the signed zero.

Approach 1: You could just acknowledge its presence in the docs. I.e. fully accept it.

Approach 2: dispose of it in the producer. Audit all implementations of real_op() in Item subclasses and call a utility function for their return value to turn the signed zero into an unsigned zero when detected.

Approach 3: dispose of it in the consumers. Every place where real_op() is called call a utility function to turn the signed zero into an unsigned zero when detected.

Which one to take is a question for the final review of course.
But at this point (before we start wasting domain expert's time) I'd like to request that you have a good story that covers more than just negation. And the story needs to be recorded into either the jira or the commit message itself.

// Normalize -0.0 to +0.0 to match binary subtraction behavior
if (res == 0.0)
res= 0.0;
Comment on lines +1830 to +1831
Copy link
Contributor

@ParadoxV5 ParadoxV5 Feb 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To a human, this looks like something a compiler might consider no-op.

Copy link
Contributor

@ParadoxV5 ParadoxV5 Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… but, checking on https://godbolt.org/, it’s not the case for either GCC -O3 or Clang -O3.

Copy link
Author

@3titoo 3titoo Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really don't know why it works, but when I remove the condition, it prints -0 when run ( select -''; ), which is the issue

return res;
}


Expand Down Expand Up @@ -1858,6 +1862,9 @@ my_decimal *Item_func_neg::decimal_op(my_decimal *decimal_value)
{
my_decimal2decimal(value.ptr(), decimal_value);
my_decimal_neg(decimal_value);
// Normalize -0 to +0 in decimal representation
Copy link
Member

@gkodinov gkodinov Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this? Do you actually have a test that hits this? I would be extremely surprised if you do.

if (my_decimal_is_zero(decimal_value))
decimal_value->sign(false);
return decimal_value;
}
return 0;
Expand Down