Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 40 additions & 0 deletions mysql-test/main/type_float.result
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,46 @@ select * from t1;
d1 d2
-1 0
drop table t1;
create table t1 (f float unsigned, key(f));
insert into t1 values (NULL),(0),(0.5);
select f, f = "-1" as eq from t1 order by f is null, f;
f eq
0 0
0.5 0
NULL NULL
select f from t1 where f = "-1";
f
select f from t1 ignore index (f) where f = "-1";
f
select f from t1 where f > "-1";
f
0
0.5
select f from t1 ignore index (f) where f > "-1";
f
0
0.5
drop table t1;
create table t1 (d double unsigned, key(d));
insert into t1 values (NULL),(0),(0.5);
select d, d = "-1" as eq from t1 order by d is null, d;
d eq
0 0
0.5 0
NULL NULL
select d from t1 where d = "-1";
d
select d from t1 ignore index (d) where d = "-1";
d
select d from t1 where d > "-1";
d
0
0.5
select d from t1 ignore index (d) where d > "-1";
d
0
0.5
drop table t1;
create table t1 (f float(4,3));
insert ignore into t1 values (-11.0),(-11),("-11"),(11.0),(11),("11");
Warnings:
Expand Down
20 changes: 20 additions & 0 deletions mysql-test/main/type_float.test
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,26 @@ update ignore t1 set d2 = d1;
select * from t1;
drop table t1;

# Do not use a truncated negative string constant for an indexed
# FLOAT/DOUBLE UNSIGNED equality range.
create table t1 (f float unsigned, key(f));
insert into t1 values (NULL),(0),(0.5);
select f, f = "-1" as eq from t1 order by f is null, f;
select f from t1 where f = "-1";
select f from t1 ignore index (f) where f = "-1";
select f from t1 where f > "-1";
select f from t1 ignore index (f) where f > "-1";
drop table t1;

create table t1 (d double unsigned, key(d));
insert into t1 values (NULL),(0),(0.5);
select d, d = "-1" as eq from t1 order by d is null, d;
select d from t1 where d = "-1";
select d from t1 ignore index (d) where d = "-1";
select d from t1 where d > "-1";
select d from t1 ignore index (d) where d > "-1";
drop table t1;

# Ensure that maximum values as the result of number of decimals
# being specified in table schema are enforced (Bug #7361)
create table t1 (f float(4,3));
Expand Down
12 changes: 12 additions & 0 deletions sql/opt_range.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9665,6 +9665,18 @@ SEL_ARG *Field_num::get_mm_leaf(RANGE_OPT_PARAM *prm, KEY_PART *key_part,
if (can_optimize_scalar_range(prm, key_part, cond, op, value) !=
Data_type_compatibility::OK)
DBUG_RETURN(0);
if (unsigned_flag &&
(type() == MYSQL_TYPE_FLOAT || type() == MYSQL_TYPE_DOUBLE))
Comment on lines +9668 to +9669

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The same clipping behavior and range construction bug also affects UNSIGNED DECIMAL columns (MYSQL_TYPE_NEWDECIMAL and MYSQL_TYPE_DECIMAL). When a negative constant is compared to an UNSIGNED DECIMAL column, it is clipped to 0 during save_in_field_no_warnings, which can lead to incorrect equality ranges being constructed for 0.

Consider extending this check to cover MYSQL_TYPE_NEWDECIMAL and MYSQL_TYPE_DECIMAL as well.

  if (unsigned_flag &&
      (type() == MYSQL_TYPE_FLOAT || type() == MYSQL_TYPE_DOUBLE ||
       type() == MYSQL_TYPE_NEWDECIMAL || type() == MYSQL_TYPE_DECIMAL))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the review. I checked this locally before extending the patch, but I could not reproduce the wrong-result behavior for DECIMAL UNSIGNED.

I tested both DECIMAL(10,2) UNSIGNED and DECIMAL(10,0) UNSIGNED with an index and compared indexed vs IGNORE INDEX execution for =, <, <=, >, and >= against the negative string constant '-1'. The indexed and non-indexed results matched in all cases. For equality, EXPLAIN showed a ref access on the decimal key with rows=0, and the query returned an empty result.

The code path also seems different from FLOAT/DOUBLE: Field_new_decimal::store_value() detects a negative value for an unsigned decimal field, sets an out-of-range warning/error, and returns a non-OK status. That lets range construction go through the existing truncated/impossible handling. The FLOAT/DOUBLE bug fixed here is that the negative value is clipped to 0 without the same error signal, so the optimizer can build an equality range for 0.

So I would prefer to keep this patch scoped to FLOAT/DOUBLE unless there is a separate DECIMAL reproducer.

{
double item_val= value->val_real();
if (!value->null_value && item_val < 0)
{
if (op == SCALAR_CMP_EQ || op == SCALAR_CMP_EQUAL ||
op == SCALAR_CMP_LT || op == SCALAR_CMP_LE)
DBUG_RETURN(new (prm->mem_root) SEL_ARG_IMPOSSIBLE(this));
DBUG_RETURN(0);
}
}
int err= value->save_in_field_no_warnings(this, 1);
if ((op != SCALAR_CMP_EQUAL && is_real_null()) || err < 0)
DBUG_RETURN(&null_element);
Expand Down