maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #10361
Please review MDEV-12001 Split Item_func_round::fix_length_and_dec to virtual methods in Type_handler
Hello Alexey,
Can you please review a patch for MDEV-12001?
Thanks!
commit c9fecdcabb9097d82dfea565a69a2f3856815111
Author: Alexander Barkov <bar@xxxxxxxxxxx>
Date: Mon Feb 6 08:43:07 2017 +0400
MDEV-12001 Split Item_func_round::fix_length_and_dec to virtual methods in Type_handler
This patch makes the following changes (according to the task description):
- Adds Type_handler::Item_func_round_fix_length_and_dec().
- Splits the code from Item_func_round::fix_length_and_dec() into new
Item_func_round methods fix_arg_int(), fix_arg_decimal(), fix_arg_double().
- Calls the new Item_func_round methods from the relevant implementations of
Type_handler_xxx::Item_func_round_fix_length_and_dec().
- Adds a new error message ER_ILLEGAL_PARAMETER_DATA_TYPE_FOR_OPERATION
- Makes ROUND() return the new error for GEOMETRY
Additionally:
- Inherits Item_func_round directly from Item_func_numhybrid as it
uses nothing from Item_func_num1.
- Fixes "MDEV-12000 ROUND(expr,const_expr_returning_NULL) creates DOUBLE(0,0)".
Now if args[1] returns NULL, the data type is set to DOUBLE with
NOT_FIXED_DEC decimals instead of 0 decimals.
diff --git a/mysql-test/r/func_math.result b/mysql-test/r/func_math.result
index f02776d..f26d1e4 100644
--- a/mysql-test/r/func_math.result
+++ b/mysql-test/r/func_math.result
@@ -817,3 +817,21 @@ select 0=0, 0=-0, 0.0= -0.0, 0.0 = -(0.0), 0.0E1=-0.0E1, 0.0E1=-(0.0E1);
select CRC32(NULL), CRC32(''), CRC32('MySQL'), CRC32('mysql'), CRC32('01234567'), CRC32('012345678');
CRC32(NULL) CRC32('') CRC32('MySQL') CRC32('mysql') CRC32('01234567') CRC32('012345678')
NULL 0 3259397556 2501908538 763378421 939184570
+#
+# Start of 10.3 tests
+#
+#
+# MDEV-12000 ROUND(expr,const_expr_returning_NULL) creates DOUBLE(0,0)
+#
+CREATE OR REPLACE TABLE t1 AS SELECT
+ROUND(10,NULL) AS c1,
+ROUND(10.1,NULL) AS c2,
+ROUND(10e0,NULL) AS c3;
+SHOW CREATE TABLE t1;
+Table Create Table
+t1 CREATE TABLE `t1` (
+ `c1` double DEFAULT NULL,
+ `c2` double DEFAULT NULL,
+ `c3` double DEFAULT NULL
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+DROP TABLE t1;
diff --git a/mysql-test/r/gis.result b/mysql-test/r/gis.result
index 3da7682..1cf8f21 100644
--- a/mysql-test/r/gis.result
+++ b/mysql-test/r/gis.result
@@ -3711,5 +3711,14 @@ CASE a WHEN POINT(1,1) THEN "a" WHEN POINT(1,2) THEN "b" END
DROP PROCEDURE p1;
DROP PROCEDURE p2;
#
+# MDEV-12001 Split Item_func_round::fix_length_and_dec to virtual methods in Type_handler
+#
+CREATE TABLE t1 (a GEOMETRY);
+SELECT ROUND(a) FROM t1;
+ERROR HY000: Illegal parameter data type geometry for operation 'round'
+DROP TABLE t1;
+SELECT ROUND(POINT(1,1));
+ERROR HY000: Illegal parameter data type geometry for operation 'round'
+#
# End of 10.3 tests
#
diff --git a/mysql-test/t/func_math.test b/mysql-test/t/func_math.test
index 08349f0..dde6e2e 100644
--- a/mysql-test/t/func_math.test
+++ b/mysql-test/t/func_math.test
@@ -608,3 +608,19 @@ select 0=0, 0=-0, 0.0= -0.0, 0.0 = -(0.0), 0.0E1=-0.0E1, 0.0E1=-(0.0E1);
--echo #
select CRC32(NULL), CRC32(''), CRC32('MySQL'), CRC32('mysql'), CRC32('01234567'), CRC32('012345678');
+
+
+--echo #
+--echo # Start of 10.3 tests
+--echo #
+
+--echo #
+--echo # MDEV-12000 ROUND(expr,const_expr_returning_NULL) creates DOUBLE(0,0)
+--echo #
+
+CREATE OR REPLACE TABLE t1 AS SELECT
+ ROUND(10,NULL) AS c1,
+ ROUND(10.1,NULL) AS c2,
+ ROUND(10e0,NULL) AS c3;
+SHOW CREATE TABLE t1;
+DROP TABLE t1;
diff --git a/mysql-test/t/gis.test b/mysql-test/t/gis.test
index 7a1ddb4..7d6c3ba 100644
--- a/mysql-test/t/gis.test
+++ b/mysql-test/t/gis.test
@@ -1868,6 +1868,18 @@ CALL p1('CREATE TABLE t1 (a Point, b Point, c Point)');
DROP PROCEDURE p1;
DROP PROCEDURE p2;
+
+--echo #
+--echo # MDEV-12001 Split Item_func_round::fix_length_and_dec to virtual methods in Type_handler
+--echo #
+
+CREATE TABLE t1 (a GEOMETRY);
+--error ER_ILLEGAL_PARAMETER_DATA_TYPE_FOR_OPERATION
+SELECT ROUND(a) FROM t1;
+DROP TABLE t1;
+--error ER_ILLEGAL_PARAMETER_DATA_TYPE_FOR_OPERATION
+SELECT ROUND(POINT(1,1));
+
--echo #
--echo # End of 10.3 tests
--echo #
diff --git a/sql/item.h b/sql/item.h
index f9f7515..56bd8c5 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -842,6 +842,21 @@ class Item: public Value_source,
*/
inline ulonglong val_uint() { return (ulonglong) val_int(); }
/*
+ Adjust the result of val_int() to an unsigned number:
+ - NULL value is converted to 0. The caller can check "null_value"
+ to distinguish between 0 and NULL when necessary.
+ - Negative numbers are converted to 0.
+ - Positive numbers bigger than upper_bound are converted to upper_bound.
+ - Other numbers are returned as is.
+ */
+ ulonglong val_uint_from_val_int(ulonglong upper_bound)
+ {
+ longlong nr= val_int();
+ return (null_value || (nr < 0 && !unsigned_flag)) ? 0 :
+ (ulonglong) nr > upper_bound ? upper_bound : (ulonglong) nr;
+ }
+
+ /*
Return string representation of this item object.
SYNOPSIS
diff --git a/sql/item_func.cc b/sql/item_func.cc
index 4725808..6dc1e27 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -2347,85 +2347,92 @@ my_decimal *Item_func_floor::decimal_op(my_decimal *decimal_value)
}
-void Item_func_round::fix_length_and_dec()
+void Item_func_round::fix_length_and_dec_decimal(uint decimals_to_set)
{
- int decimals_to_set;
- longlong val1;
- bool val1_unsigned;
-
+ DBUG_ASSERT(decimals_to_set <= DECIMAL_MAX_SCALE);
+ set_handler_by_result_type(DECIMAL_RESULT);
+ unsigned_flag= args[0]->unsigned_flag;
+ decimals= decimals_to_set;
+ int decimals_delta= args[0]->decimals - decimals_to_set;
+ int precision= args[0]->decimal_precision();
+ int length_increase= ((decimals_delta <= 0) || truncate) ? 0 : 1;
+ precision-= decimals_delta - length_increase;
+ max_length= my_decimal_precision_to_length_no_truncation(precision,
+ decimals,
+ unsigned_flag);
+}
+
+
+void Item_func_round::fix_length_and_dec_double(uint decimals_to_set)
+{
+ set_handler(&type_handler_double);
unsigned_flag= args[0]->unsigned_flag;
+ decimals= decimals_to_set;
+ max_length= float_length(decimals_to_set);
+}
+
+
+void Item_func_round::fix_arg_decimal()
+{
if (!args[1]->const_item())
{
+ set_handler(&type_handler_newdecimal);
+ unsigned_flag= args[0]->unsigned_flag;
decimals= args[0]->decimals;
- max_length= float_length(decimals);
- if (args[0]->result_type() == DECIMAL_RESULT)
- {
- max_length++;
- set_handler_by_result_type(DECIMAL_RESULT);
- }
+ max_length= float_length(args[0]->decimals) + 1;
+ }
+ else
+ {
+ uint dec= (uint) args[1]->val_uint_from_val_int(DECIMAL_MAX_SCALE);
+ if (args[1]->null_value)
+ fix_length_and_dec_double(NOT_FIXED_DEC);
else
- set_handler_by_result_type(REAL_RESULT);
- return;
+ fix_length_and_dec_decimal(dec);
}
+}
- val1= args[1]->val_int();
- if ((null_value= args[1]->null_value))
- return;
- val1_unsigned= args[1]->unsigned_flag;
- if (val1 < 0)
- decimals_to_set= val1_unsigned ? INT_MAX : 0;
+void Item_func_round::fix_arg_double()
+{
+ if (!args[1]->const_item())
+ fix_length_and_dec_double(args[0]->decimals);
else
- decimals_to_set= (val1 > INT_MAX) ? INT_MAX : (int) val1;
-
- if (args[0]->decimals == NOT_FIXED_DEC)
{
- decimals= MY_MIN(decimals_to_set, NOT_FIXED_DEC);
- max_length= float_length(decimals);
- set_handler_by_result_type(REAL_RESULT);
- return;
+ uint dec= (uint) args[1]->val_uint_from_val_int(NOT_FIXED_DEC);
+ fix_length_and_dec_double(args[1]->null_value ? NOT_FIXED_DEC : dec);
}
-
- switch (args[0]->result_type()) {
- case REAL_RESULT:
- case STRING_RESULT:
- set_handler_by_result_type(REAL_RESULT);
- decimals= MY_MIN(decimals_to_set, NOT_FIXED_DEC);
- max_length= float_length(decimals);
- break;
- case INT_RESULT:
- if ((!decimals_to_set && truncate) || (args[0]->decimal_precision() < DECIMAL_LONGLONG_DIGITS))
+}
+
+
+void Item_func_round::fix_arg_int()
+{
+ if (!args[1]->const_item())
+ fix_length_and_dec_double(args[0]->decimals);
+ else
+ {
+ longlong val1= args[1]->val_int();
+ bool val1_is_negative= val1 < 0 && !args[1]->unsigned_flag;
+ uint decimals_to_set= val1_is_negative ?
+ 0 : (uint) MY_MIN(val1, DECIMAL_MAX_SCALE);
+ if (args[1]->null_value)
+ fix_length_and_dec_double(NOT_FIXED_DEC);
+ else if ((!decimals_to_set && truncate) ||
+ args[0]->decimal_precision() < DECIMAL_LONGLONG_DIGITS)
{
- int length_can_increase= MY_TEST(!truncate && (val1 < 0) &&
- !val1_unsigned);
+ // Length can increase in some cases: ROUND(9,-1) -> 10
+ int length_can_increase= MY_TEST(!truncate && val1_is_negative);
max_length= args[0]->max_length + length_can_increase;
- /* Here we can keep INT_RESULT */
+ // Here we can keep INT_RESULT
set_handler_by_result_type(INT_RESULT);
+ unsigned_flag= args[0]->unsigned_flag;
decimals= 0;
- break;
}
- /* fall through */
- case DECIMAL_RESULT:
- {
- set_handler_by_result_type(DECIMAL_RESULT);
- decimals_to_set= MY_MIN(DECIMAL_MAX_SCALE, decimals_to_set);
- int decimals_delta= args[0]->decimals - decimals_to_set;
- int precision= args[0]->decimal_precision();
- int length_increase= ((decimals_delta <= 0) || truncate) ? 0:1;
-
- precision-= decimals_delta - length_increase;
- decimals= MY_MIN(decimals_to_set, DECIMAL_MAX_SCALE);
- max_length= my_decimal_precision_to_length_no_truncation(precision,
- decimals,
- unsigned_flag);
- break;
- }
- case ROW_RESULT:
- case TIME_RESULT:
- DBUG_ASSERT(0); /* This result type isn't handled */
+ else
+ fix_length_and_dec_decimal(decimals_to_set);
}
}
+
double my_double_round(double value, longlong dec, bool dec_unsigned,
bool truncate)
{
diff --git a/sql/item_func.h b/sql/item_func.h
index fe0b766..42fd527 100644
--- a/sql/item_func.h
+++ b/sql/item_func.h
@@ -1202,17 +1202,25 @@ class Item_func_floor :public Item_func_int_val
/* This handles round and truncate */
-class Item_func_round :public Item_func_num1
+class Item_func_round :public Item_func_numhybrid
{
bool truncate;
+ void fix_length_and_dec_decimal(uint decimals_to_set);
+ void fix_length_and_dec_double(uint decimals_to_set);
public:
Item_func_round(THD *thd, Item *a, Item *b, bool trunc_arg)
- :Item_func_num1(thd, a, b), truncate(trunc_arg) {}
+ :Item_func_numhybrid(thd, a, b), truncate(trunc_arg) {}
const char *func_name() const { return truncate ? "truncate" : "round"; }
double real_op();
longlong int_op();
my_decimal *decimal_op(my_decimal *);
- void fix_length_and_dec();
+ void fix_arg_decimal();
+ void fix_arg_int();
+ void fix_arg_double();
+ void fix_length_and_dec()
+ {
+ args[0]->type_handler()->Item_func_round_fix_length_and_dec(this);
+ }
Item *get_copy(THD *thd, MEM_ROOT *mem_root)
{ return get_item_copy<Item_func_round>(thd, mem_root, this); }
};
diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
index a075764..c0796b2 100644
--- a/sql/share/errmsg-utf8.txt
+++ b/sql/share/errmsg-utf8.txt
@@ -7450,3 +7450,5 @@ ER_JSON_PATH_EMPTY
eng "Path expression '$' is not allowed in argument %d to function '%s'."
ER_ILLEGAL_PARAMETER_DATA_TYPES2_FOR_OPERATION
eng "Illegal parameter data types %s and %s for operation '%s'"
+ER_ILLEGAL_PARAMETER_DATA_TYPE_FOR_OPERATION
+ eng "Illegal parameter data type %s for operation '%s'"
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index d165470..6de1af2 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -26,7 +26,6 @@ static Type_handler_long type_handler_long;
static Type_handler_int24 type_handler_int24;
static Type_handler_year type_handler_year;
static Type_handler_float type_handler_float;
-static Type_handler_double type_handler_double;
static Type_handler_time type_handler_time;
static Type_handler_time2 type_handler_time2;
static Type_handler_date type_handler_date;
@@ -51,6 +50,7 @@ Type_handler_null type_handler_null;
Type_handler_row type_handler_row;
Type_handler_varchar type_handler_varchar;
Type_handler_longlong type_handler_longlong;
+Type_handler_double type_handler_double;
Type_handler_newdecimal type_handler_newdecimal;
Type_handler_datetime type_handler_datetime;
Type_handler_bit type_handler_bit;
@@ -2155,3 +2155,63 @@ String *Type_handler_timestamp_common::
/***************************************************************************/
+
+bool Type_handler_row::
+ Item_func_round_fix_length_and_dec(Item_func_round *item) const
+{
+ DBUG_ASSERT(0);
+ return false;
+}
+
+
+bool Type_handler_int_result::
+ Item_func_round_fix_length_and_dec(Item_func_round *item) const
+{
+ item->fix_arg_int();
+ return false;
+}
+
+
+bool Type_handler_real_result::
+ Item_func_round_fix_length_and_dec(Item_func_round *item) const
+{
+ item->fix_arg_double();
+ return false;
+}
+
+
+bool Type_handler_decimal_result::
+ Item_func_round_fix_length_and_dec(Item_func_round *item) const
+{
+ item->fix_arg_decimal();
+ return false;
+}
+
+
+bool Type_handler_temporal_result::
+ Item_func_round_fix_length_and_dec(Item_func_round *item) const
+{
+ item->fix_arg_double();
+ return false;
+}
+
+
+bool Type_handler_string_result::
+ Item_func_round_fix_length_and_dec(Item_func_round *item) const
+{
+ item->fix_arg_double();
+ return false;
+}
+
+
+#ifdef HAVE_SPATIAL
+bool Type_handler_geometry::
+ Item_func_round_fix_length_and_dec(Item_func_round *item) const
+{
+ my_error(ER_ILLEGAL_PARAMETER_DATA_TYPE_FOR_OPERATION, MYF(0),
+ type_handler_geometry.name().ptr(), item->func_name());
+ return false;
+}
+#endif
+
+/***************************************************************************/
diff --git a/sql/sql_type.h b/sql/sql_type.h
index 79cdba9..45ffef5 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -35,6 +35,7 @@ class Item_func_hybrid_field_type;
class Item_bool_func2;
class Item_func_between;
class Item_func_in;
+class Item_func_round;
class cmp_item;
class in_vector;
class Type_std_attributes;
@@ -443,6 +444,9 @@ class Type_handler
virtual bool
Item_func_in_fix_comparator_compatible_types(THD *thd, Item_func_in *)
const= 0;
+
+ virtual bool
+ Item_func_round_fix_length_and_dec(Item_func_round *round) const= 0;
};
@@ -594,6 +598,7 @@ class Type_handler_row: public Type_handler
in_vector *make_in_vector(THD *thd, const Item_func_in *f, uint nargs) const;
bool Item_func_in_fix_comparator_compatible_types(THD *thd,
Item_func_in *) const;
+ bool Item_func_round_fix_length_and_dec(Item_func_round *) const;
};
@@ -662,6 +667,7 @@ class Type_handler_real_result: public Type_handler_numeric
bool Item_func_in_fix_comparator_compatible_types(THD *thd,
Item_func_in *) const;
+ bool Item_func_round_fix_length_and_dec(Item_func_round *) const;
};
@@ -704,6 +710,7 @@ class Type_handler_decimal_result: public Type_handler_numeric
in_vector *make_in_vector(THD *, const Item_func_in *, uint nargs) const;
bool Item_func_in_fix_comparator_compatible_types(THD *thd,
Item_func_in *) const;
+ bool Item_func_round_fix_length_and_dec(Item_func_round *) const;
};
@@ -745,6 +752,7 @@ class Type_handler_int_result: public Type_handler_numeric
in_vector *make_in_vector(THD *, const Item_func_in *, uint nargs) const;
bool Item_func_in_fix_comparator_compatible_types(THD *thd,
Item_func_in *) const;
+ bool Item_func_round_fix_length_and_dec(Item_func_round *) const;
};
@@ -790,6 +798,7 @@ class Type_handler_temporal_result: public Type_handler
longlong Item_func_between_val_int(Item_func_between *func) const;
bool Item_func_in_fix_comparator_compatible_types(THD *thd,
Item_func_in *) const;
+ bool Item_func_round_fix_length_and_dec(Item_func_round *) const;
};
@@ -849,6 +858,7 @@ class Type_handler_string_result: public Type_handler
in_vector *make_in_vector(THD *, const Item_func_in *, uint nargs) const;
bool Item_func_in_fix_comparator_compatible_types(THD *thd,
Item_func_in *) const;
+ bool Item_func_round_fix_length_and_dec(Item_func_round *) const;
};
@@ -1263,6 +1273,7 @@ class Type_handler_geometry: public Type_handler_string_result
{
return false;
}
+ bool Item_func_round_fix_length_and_dec(Item_func_round *) const;
};
#endif
@@ -1381,6 +1392,7 @@ extern Type_handler_row type_handler_row;
extern Type_handler_null type_handler_null;
extern Type_handler_varchar type_handler_varchar;
extern Type_handler_longlong type_handler_longlong;
+extern Type_handler_double type_handler_double;
extern Type_handler_newdecimal type_handler_newdecimal;
extern Type_handler_datetime type_handler_datetime;
extern Type_handler_longlong type_handler_longlong;
