MySQL Bugs: #60176: max(if(1,unsigned,signed)) inconsistent

Bug #60176	max(if(1,unsigned,signed)) inconsistent
Submitted:	18 Feb 2011 22:23	Modified:	19 Feb 2011 9:35
Reporter:	Sergei Golubchik	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: General	Severity:	S3 (Non-critical)
Version:	5.1	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
agg_field_type() and agg_result_type() sometimes aggregate differently. In particular, in Item_func_if::fix_length_and_dec() they may disagree and we get cached_result_type=DECIMAL_RESULT but cached_field_type=MYSQL_TYPE_LONGLONG. This shows up if the result of IF() is passed to another comparison function.

How to repeat:
The test straight from func_if.test:

CREATE TABLE t1 (c LONGTEXT);
INSERT INTO t1 VALUES(1), (2), (3), (4), ('12345678901234567890');
SELECT MAX(IF(1, CAST(c AS UNSIGNED), 0)) FROM t1;
SELECT IF(1, CAST(c AS UNSIGNED), 0) FROM t1;

Here, MAX() compares values as DECIMAL, and unsigned number 12345678901234567890 is converted into decimal without modifications. Thus it's truly the largest number in the set.

But if you strip away MAX(), then IF() will report its result as being signed BIGINT. 12345678901234567890 is converted to a signed bigint, it's wrapped around and becomes -6101065172474983726. Suddenly it's not largest anymore.

Suggested fix:
Fix either agg_field_type() or agg_result_type() to agree with each other.

Thank you for the bug report. Verified on Mac OS X:

macbook-pro:5.1 openxs$ bin/mysql -uroot --column-type-info test
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 1
Server version: 5.1.56-debug Source distribution

Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
This software comes with ABSOLUTELY NO WARRANTY. This is free software,
and you are welcome to modify and redistribute it under the GPL v2 license

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> drop table t1;
Query OK, 0 rows affected (0.06 sec)

mysql> CREATE TABLE t1 (c LONGTEXT);
Query OK, 0 rows affected (0.10 sec)

mysql> INSERT INTO t1 VALUES(1), (2), (3), (4), ('12345678901234567890');
Query OK, 5 rows affected (0.01 sec)
Records: 5  Duplicates: 0  Warnings: 0

mysql> SELECT MAX(IF(1, CAST(c AS UNSIGNED), 0)) FROM t1;
Field   1:  `MAX(IF(1, CAST(c AS UNSIGNED), 0))`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       NEWDECIMAL
Collation:  binary (63)
Length:     68
Max_length: 20
Decimals:   0
Flags:      BINARY 

+------------------------------------+
| MAX(IF(1, CAST(c AS UNSIGNED), 0)) |
+------------------------------------+
|               12345678901234567890 |
+------------------------------------+
1 row in set (0.00 sec)

mysql> SELECT IF(1, CAST(c AS UNSIGNED), 0) FROM t1;
Field   1:  `IF(1, CAST(c AS UNSIGNED), 0)`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       LONGLONG
Collation:  binary (63)
Length:     68
Max_length: 20
Decimals:   0
Flags:      BINARY NUM 

+-------------------------------+
| IF(1, CAST(c AS UNSIGNED), 0) |
+-------------------------------+
|                             1 |
|                             2 |
|                             3 |
|                             4 |
|          -6101065172474983726 |
+-------------------------------+
5 rows in set (0.01 sec)

The problem is, in case of the if function, that the unsigned flag is determined incorrectly. To solve the problem, the unsighed_flag variable in the Item_func_if::fix_length_and_dec() function (Item_cmpfunc.cc) must be determined as the result of a logical disjunction performed on the unsigned flags of the arguments:

  unsigned_flag=args[1]->unsigned_flag || args[2]->unsigned_flag;

rather than a logical conjunction, as we have it now: 

  unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag;

=== modified file 'sql/item_cmpfunc.cc'
--- item_cmpfunc.cc     2011-03-31 18:59:11 +0000
+++ item_cmpfunc.cc     2011-06-16 10:15:03 +0000
@@ -0,0 +1,17 @@ void
Item_func_if::fix_length_and_dec()
{
  maybe_null=args[1]->maybe_null || args[2]->maybe_null;
  decimals= max(args[1]->decimals, args[2]->decimals);
- unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag;
+ unsigned_flag=args[1]->unsigned_flag || args[2]->unsigned_flag;

}

Bug #61540 was marked as a duplicate of this one.

The above solution will not work, though, if the second argument is negative, which may happen, of course, as long as we're talking about a signed type. My mistake. 

A simple workaround can be using a bit more sophisticated algorithm of determining the unsigned flag, as follows: 

=== modified file 'sql/item_cmpfunc.cc'
--- item_cmpfunc.cc     2011-03-31 18:59:11 +0000
+++ item_cmpfunc.cc     2011-06-22 11:39:59 +0000
@@ -0,0 +1,12 @@ void
Item_func_if::fix_length_and_dec()
{

  unsigned_flag=args[1]->unsigned_flag && args[2]->unsigned_flag;

+  if ((args[0]->val_int() && args[1]->unsigned_flag)||(!args[0]->val_int() && args[2]->unsigned_flag))
+    unsigned_flag=1;

}

BTW, The same approach is used in MAX(). Run, for example, the following query:

select max(cast(c as unsigned)) from t1;

Field   1:  `max(cast(c as unsigned))`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       LONGLONG
Collation:  binary (63)
Length:     20
Max_length: 20
Decimals:   0
Flags:      UNSIGNED BINARY NUM 

+--------------------------+
| max(cast(c as unsigned)) |
+--------------------------+
|     12345678901234567890 |
+--------------------------+

As you can see, it generates the correct result in spite of the fact that the field_type is LONGLONG. What is important though the unsigned flag is set.