← Back to team overview

maria-developers team mailing list archive

MDEV-8109 unexpected CAST result

 

Hi Serg,

Please review my patch for MDEV-8109.

Thanks!
diff --git a/mysql-test/r/strict.result b/mysql-test/r/strict.result
index 9dcd597..06f0869 100644
--- a/mysql-test/r/strict.result
+++ b/mysql-test/r/strict.result
@@ -1117,11 +1117,14 @@ ERROR 22007: Truncated incorrect CHAR(3) value: '1000'
 insert into t1 (col1) values (cast(1000.0 as char(3)));
 ERROR 22007: Truncated incorrect CHAR(3) value: '1000.0'
 insert into t1 (col2) values (cast('abc' as signed integer));
-ERROR 22007: Truncated incorrect INTEGER value: 'abc'
+Warnings:
+Warning	1292	Truncated incorrect INTEGER value: 'abc'
 insert into t1 (col2) values (10E+0 + 'a');
-ERROR 22007: Truncated incorrect DOUBLE value: 'a'
+Warnings:
+Warning	1292	Truncated incorrect DOUBLE value: 'a'
 insert into t1 (col2) values (cast('10a' as unsigned integer));
-ERROR 22007: Truncated incorrect INTEGER value: '10a'
+Warnings:
+Warning	1292	Truncated incorrect INTEGER value: '10a'
 insert into t1 (col2) values (cast('10' as unsigned integer));
 insert into t1 (col2) values (cast('10' as signed integer));
 insert into t1 (col2) values (10E+0 + '0 ');
@@ -1129,6 +1132,9 @@ Warnings:
 Note	1292	Truncated incorrect DOUBLE value: '0 '
 select * from t1;
 col1	col2
+NULL	0
+NULL	10
+NULL	10
 NULL	10
 NULL	10
 NULL	10
@@ -1527,3 +1533,26 @@ Warning	1411	Incorrect datetime value: '2001' for function str_to_date
 #
 # End of 5.6 tests
 #
+#
+# Start of 10.1 tests
+#
+#
+# MDEV-8300 CAST('' AS DECIMAL) is too strict on INSERT in strict mode
+#
+SET sql_mode='STRICT_ALL_TABLES';
+CREATE TABLE t1 (a DECIMAL,b INT, c DOUBLE);
+INSERT INTO t1 VALUES(CAST('' AS DECIMAL),CAST('' AS SIGNED),CAST('' AS DOUBLE));
+Warnings:
+Warning	1292	Truncated incorrect DECIMAL value: ''
+Warning	1292	Truncated incorrect INTEGER value: ''
+Warning	1292	Truncated incorrect DOUBLE value: ''
+INSERT INTO t1 VALUES(''+0,''+0,''+0);
+Warnings:
+Warning	1292	Truncated incorrect DOUBLE value: ''
+Warning	1292	Truncated incorrect DOUBLE value: ''
+Warning	1292	Truncated incorrect DOUBLE value: ''
+DROP TABLE t1;
+SET sql_mode=DEFAULT;
+#
+# End of 10.1 tests
+#
diff --git a/mysql-test/suite/vcol/r/vcol_misc.result b/mysql-test/suite/vcol/r/vcol_misc.result
index 279777b..124d901 100644
--- a/mysql-test/suite/vcol/r/vcol_misc.result
+++ b/mysql-test/suite/vcol/r/vcol_misc.result
@@ -365,5 +365,40 @@ aaa
 Warnings:
 Warning	1918	Encountered illegal value '\xF0\xF1\xF2\xF3\xF4\xF5\xF6\xF7' when converting to DOUBLE
 #
+# MDEV-8109 unexpected CAST result
+#
+SET sql_mode=STRICT_ALL_TABLES;
+CREATE FUNCTION test(par_aaa DECIMAL(10,2))
+RETURNS DECIMAL(10,2) DETERMINISTIC
+RETURN 0;
+SET @aaa= COLUMN_CREATE('price', '');
+SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) AS aaa;
+aaa
+0
+Warnings:
+Warning	1918	Encountered illegal value '' when converting to DECIMAL
+SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) + 1 AS aaa;
+aaa
+1
+Warnings:
+Warning	1918	Encountered illegal value '' when converting to DECIMAL
+SELECT test(COLUMN_GET(@aaa, 'price' AS DECIMAL)) AS bbb;
+bbb
+0.00
+Warnings:
+Warning	1918	Encountered illegal value '' when converting to DECIMAL
+SELECT test(CAST('' AS DECIMAL(10,2))) AS bbb;
+bbb
+0.00
+Warnings:
+Warning	1292	Truncated incorrect DECIMAL value: ''
+SELECT test(''+0) AS bbb;
+bbb
+0.00
+Warnings:
+Warning	1292	Truncated incorrect DOUBLE value: ''
+DROP FUNCTION test;
+SET sql_mode=DEFAULT;
+#
 # End of 10.1 tests
 #
diff --git a/mysql-test/suite/vcol/t/vcol_misc.test b/mysql-test/suite/vcol/t/vcol_misc.test
index c5b534f..e5951ec 100644
--- a/mysql-test/suite/vcol/t/vcol_misc.test
+++ b/mysql-test/suite/vcol/t/vcol_misc.test
@@ -310,6 +310,23 @@ SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) aaa;
 SELECT COLUMN_GET(@aaa, 'price' AS INT) aaa;
 SELECT COLUMN_GET(@aaa, 'price' AS DOUBLE) aaa;
 
+--echo #
+--echo # MDEV-8109 unexpected CAST result
+--echo #
+SET sql_mode=STRICT_ALL_TABLES;
+
+CREATE FUNCTION test(par_aaa DECIMAL(10,2))
+  RETURNS DECIMAL(10,2) DETERMINISTIC
+  RETURN 0;
+SET @aaa= COLUMN_CREATE('price', '');
+SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) AS aaa;
+SELECT COLUMN_GET(@aaa, 'price' AS DECIMAL) + 1 AS aaa;
+SELECT test(COLUMN_GET(@aaa, 'price' AS DECIMAL)) AS bbb;
+SELECT test(CAST('' AS DECIMAL(10,2))) AS bbb;
+SELECT test(''+0) AS bbb;
+DROP FUNCTION test;
+
+SET sql_mode=DEFAULT;
 
 --echo #
 --echo # End of 10.1 tests
diff --git a/mysql-test/t/strict.test b/mysql-test/t/strict.test
index 93b3149..4414703 100644
--- a/mysql-test/t/strict.test
+++ b/mysql-test/t/strict.test
@@ -992,11 +992,8 @@ insert into t1 (col1) values (cast(1000 as char(3)));
 insert into t1 (col1) values (cast(1000E+0 as char(3)));
 --error 1292
 insert into t1 (col1) values (cast(1000.0 as char(3)));
---error 1292
 insert into t1 (col2) values (cast('abc' as signed integer));
---error 1292
 insert into t1 (col2) values (10E+0 + 'a');
---error 1292
 insert into t1 (col2) values (cast('10a' as unsigned integer));
 insert into t1 (col2) values (cast('10' as unsigned integer));
 insert into t1 (col2) values (cast('10' as signed integer));
@@ -1370,3 +1367,21 @@ SELECT STR_TO_DATE('2001','%Y'),CONCAT(STR_TO_DATE('2001','%Y')), STR_TO_DATE('2
 --echo #
 --echo # End of 5.6 tests
 --echo #
+
+--echo #
+--echo # Start of 10.1 tests
+--echo #
+
+--echo #
+--echo # MDEV-8300 CAST('' AS DECIMAL) is too strict on INSERT in strict mode
+--echo #
+SET sql_mode='STRICT_ALL_TABLES';
+CREATE TABLE t1 (a DECIMAL,b INT, c DOUBLE);
+INSERT INTO t1 VALUES(CAST('' AS DECIMAL),CAST('' AS SIGNED),CAST('' AS DOUBLE));
+INSERT INTO t1 VALUES(''+0,''+0,''+0);
+DROP TABLE t1;
+SET sql_mode=DEFAULT;
+
+--echo #
+--echo # End of 10.1 tests
+--echo #
diff --git a/sql/field.cc b/sql/field.cc
index 5b33d80..18d22b9 100644
--- a/sql/field.cc
+++ b/sql/field.cc
@@ -1410,10 +1410,25 @@ Value_source::Converter_string_to_number::check_edom_and_truncation(THD *thd,
         end \0 here.
       */
       THD *wthd= thd ? thd : current_thd;
-      push_warning_printf(wthd, Sql_condition::WARN_LEVEL_WARN,
-                          ER_TRUNCATED_WRONG_VALUE,
-                          ER_THD(wthd, ER_TRUNCATED_WRONG_VALUE), type,
-                          ErrConvString(str, length, cs).ptr());
+      /*
+        We use THD::raise_warning_printf() rather than ::push_warning_printf()
+        because we don't want warnings to be escalated to errors in
+        case of strict mode in a query like this:
+          INSERT INTO t1 (decimal_column) VALUES (CAST('' AS DECIMAL));
+        It works as follows:
+          - CAST returns 0 with a warning
+          - INSERT writes the result of CAST into the table normally
+
+        Notice,
+        this is different from how direct inserting of a bad value works:
+          INSERT INTO t1 (decimal_column) VALUES ('');
+        Unlike the above query with CAST, the bad value is written into the
+        field directly here, so the warning is escalated to error
+        (when in STRICT_XXX_TABLES mode).
+        See Field_num::check_edom_and_truncation().
+      */
+      wthd->raise_warning_printf(ER_TRUNCATED_WRONG_VALUE,
+                                 type, ErrConvString(str, length, cs).ptr());
     }
   }
   else if (m_end_of_num < str + length)
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index d9cb252..ec3d0ce 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -4945,11 +4945,9 @@ longlong Item_dyncol_get::val_int()
       THD *thd= current_thd;
       char buff[30];
       sprintf(buff, "%lg", val.x.double_value);
-      push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-                          ER_DATA_OVERFLOW,
-                          ER_THD(thd, ER_DATA_OVERFLOW),
-                          buff,
-                          unsigned_flag ? "UNSIGNED INT" : "INT");
+      thd->raise_warning_printf(ER_DATA_OVERFLOW,
+                                buff,
+                                unsigned_flag ? "UNSIGNED INT" : "INT");
     }
     return num;
   }
@@ -4963,13 +4961,11 @@ longlong Item_dyncol_get::val_int()
     if (end != org_end || error > 0)
     {
       THD *thd= current_thd;
-      push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-                          ER_BAD_DATA,
-                          ER_THD(thd, ER_BAD_DATA),
-                          ErrConvString(val.x.string.value.str,
-                                        val.x.string.value.length,
-                                        val.x.string.charset).ptr(),
-                          unsigned_flag ? "UNSIGNED INT" : "INT");
+      thd->raise_warning_printf(ER_BAD_DATA,
+                                ErrConvString(val.x.string.value.str,
+                                              val.x.string.value.length,
+                                              val.x.string.charset).ptr(),
+                                unsigned_flag ? "UNSIGNED INT" : "INT");
     }
     unsigned_flag= error >= 0;
     return num;
@@ -5027,13 +5023,11 @@ double Item_dyncol_get::val_real()
         error)
     {
       THD *thd= current_thd;
-      push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-                          ER_BAD_DATA,
-                          ER_THD(thd, ER_BAD_DATA),
-                          ErrConvString(val.x.string.value.str,
-                                        val.x.string.value.length,
-                                        val.x.string.charset).ptr(),
-                          "DOUBLE");
+      thd->raise_warning_printf(ER_BAD_DATA,
+                                ErrConvString(val.x.string.value.str,
+                                              val.x.string.value.length,
+                                              val.x.string.charset).ptr(),
+                                "DOUBLE");
     }
     return res;
   }
@@ -5088,13 +5082,11 @@ my_decimal *Item_dyncol_get::val_decimal(my_decimal *decimal_value)
         end != val.x.string.value.str + val.x.string.value.length)
     {
       THD *thd= current_thd;
-      push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-                          ER_BAD_DATA,
-                          ER_THD(thd, ER_BAD_DATA),
-                          ErrConvString(val.x.string.value.str,
-                                        val.x.string.value.length,
-                                        val.x.string.charset).ptr(),
-                          "DECIMAL");
+      thd->raise_warning_printf(ER_BAD_DATA,
+                                ErrConvString(val.x.string.value.str,
+                                              val.x.string.value.length,
+                                              val.x.string.charset).ptr(),
+                                "DECIMAL");
     }
     break;
   }
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 0a0d3ca..beaf080 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -1235,10 +1235,6 @@ Sql_condition* THD::raise_condition(uint sql_errno,
   Sql_condition *cond= NULL;
   DBUG_ENTER("THD::raise_condition");
 
-  if (!(variables.option_bits & OPTION_SQL_NOTES) &&
-      (level == Sql_condition::WARN_LEVEL_NOTE))
-    DBUG_RETURN(NULL);
-
   da->opt_clear_warning_info(query_id);
 
   /*
@@ -1253,17 +1249,6 @@ Sql_condition* THD::raise_condition(uint sql_errno,
   if (sqlstate == NULL)
    sqlstate= mysql_errno_to_sqlstate(sql_errno);
 
-  if ((level == Sql_condition::WARN_LEVEL_WARN) &&
-      really_abort_on_warning())
-  {
-    /*
-      FIXME:
-      push_warning and strict SQL_MODE case.
-    */
-    level= Sql_condition::WARN_LEVEL_ERROR;
-    killed= KILL_BAD_DATA;
-  }
-
   switch (level)
   {
   case Sql_condition::WARN_LEVEL_NOTE:
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 2b62c46..93c580d 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -3679,6 +3679,30 @@ class THD :public Statement,
   */
   void raise_note_printf(uint code, ...);
 
+  /**
+    Raise a warning with optional warning-to-error escalation and note
+    suppressing. See raise_condition_with_escalation for details.
+  */
+  void raise_warning_with_escalation(uint sql_errno,
+                                     const char *sqlstate,
+                                     Sql_condition::enum_warning_level level,
+                                     const char *msg)
+  {
+    /*
+      Calling push_warning/push_warning_printf with a level of
+      WARN_LEVEL_ERROR *is* a bug.  Either use my_printf_error(),
+      my_error(), or WARN_LEVEL_WARN.
+    */
+    DBUG_ASSERT(level != Sql_condition::WARN_LEVEL_ERROR);
+
+    if (level == Sql_condition::WARN_LEVEL_ERROR)
+      level= Sql_condition::WARN_LEVEL_WARN;
+
+    (void) raise_condition_with_escalation(sql_errno, NULL, level, msg);
+
+    /* Make sure we also count warnings pushed after calling set_ok_status(). */
+    get_stmt_da()->increment_warning();
+  }
 private:
   /*
     Only the implementation of the SIGNAL and RESIGNAL statements
@@ -3690,7 +3714,6 @@ class THD :public Statement,
   friend class Sql_cmd_common_signal;
   friend class Sql_cmd_signal;
   friend class Sql_cmd_resignal;
-  friend void push_warning(THD*, Sql_condition::enum_warning_level, uint, const char*);
   friend void my_message_sql(uint, const char *, myf);
 
   /**
@@ -3707,6 +3730,44 @@ class THD :public Statement,
                   Sql_condition::enum_warning_level level,
                   const char* msg);
 
+  /**
+    Raise a generic SQL condition with optional warn-to-error escalation
+    and note suppressing.
+
+    - Messages with level WARN_LEVEL_WARN are escalated to errors
+      if in strict mode (STRICT_TRANS_TABLES, STRICT_ALL_TABLES)
+      and thd->abort_on_warning is set to "true".
+    - Messages with level WARN_LEVEL_NOTE are suppressed if
+      the OPTION_SQL_NOTES is not set in variables.option_bits.
+
+    @param sql_errno the condition error number
+    @param sqlstate the condition SQLSTATE
+    @param level the condition level
+    @param msg the condition message text
+    @return The condition raised, or NULL
+  */
+  Sql_condition*
+  raise_condition_with_escalation(uint sql_errno,
+                                  const char *sqlstate,
+                                  Sql_condition::enum_warning_level level,
+                                  const char* msg)
+  {
+    if (!(variables.option_bits & OPTION_SQL_NOTES) &&
+        (level == Sql_condition::WARN_LEVEL_NOTE))
+      return NULL;
+
+    if ((level == Sql_condition::WARN_LEVEL_WARN) &&
+        really_abort_on_warning())
+    {
+      /*
+        FIXME:
+        push_warning and strict SQL_MODE case.
+      */
+      level= Sql_condition::WARN_LEVEL_ERROR;
+      killed= KILL_BAD_DATA;
+    }
+    return raise_condition(sql_errno, sqlstate, level, msg);
+  }
 public:
   /** Overloaded to guard query/query_length fields */
   virtual void set_statement(Statement *stmt);
diff --git a/sql/sql_error.cc b/sql/sql_error.cc
index 47f24ad..9c5ffaa 100644
--- a/sql/sql_error.cc
+++ b/sql/sql_error.cc
@@ -737,20 +737,7 @@ void push_warning(THD *thd, Sql_condition::enum_warning_level level,
   DBUG_ENTER("push_warning");
   DBUG_PRINT("enter", ("code: %d, msg: %s", code, msg));
 
-  /*
-    Calling push_warning/push_warning_printf with a level of
-    WARN_LEVEL_ERROR *is* a bug.  Either use my_printf_error(),
-    my_error(), or WARN_LEVEL_WARN.
-  */
-  DBUG_ASSERT(level != Sql_condition::WARN_LEVEL_ERROR);
-
-  if (level == Sql_condition::WARN_LEVEL_ERROR)
-    level= Sql_condition::WARN_LEVEL_WARN;
-
-  (void) thd->raise_condition(code, NULL, level, msg);
-
-  /* Make sure we also count warnings pushed after calling set_ok_status(). */
-  thd->get_stmt_da()->increment_warning();
+  (void) thd->raise_warning_with_escalation(code, NULL, level, msg);
 
   DBUG_VOID_RETURN;
 }
@@ -782,7 +769,7 @@ void push_warning_printf(THD *thd, Sql_condition::enum_warning_level level,
   my_vsnprintf_ex(&my_charset_utf8_general_ci, warning,
                   sizeof(warning), format, args);
   va_end(args);
-  push_warning(thd, level, code, warning);
+  (void) thd->raise_warning_with_escalation(code, NULL, level, warning);
   DBUG_VOID_RETURN;
 }
 

Follow ups